Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Proper handling of units in the user interface #183

Open
fhagemann opened this issue Jul 15, 2021 · 7 comments
Open

Proper handling of units in the user interface #183

fhagemann opened this issue Jul 15, 2021 · 7 comments
Assignees
Labels
convenience Improve user-friendliness enhancement Improvement of existing features notation Notation and Interface
Milestone

Comments

@fhagemann
Copy link
Collaborator

It would be nice to allow for units in the user interface, especially for plotting, e.g.

plot(simulation.electric_field, xlims = (-4u"cm", 4u"cm"))

This also comes with nice unit handling in configuration files (see #182).

@oschulz
Copy link
Member

oschulz commented Jul 15, 2021

We should take a look at UnitfulRecipes.jl.

@fhagemann
Copy link
Collaborator Author

I just realized this is a duplicate of #66. Closing #66 in favour of this.

@fhagemann fhagemann added convenience Improve user-friendliness enhancement Improvement of existing features notation Notation and Interface labels Jul 16, 2021
@fhagemann fhagemann added this to the v1.0.0 milestone Jul 16, 2021
@lmh91
Copy link
Collaborator

lmh91 commented Jul 23, 2021

Regarding the input and output units of the signal generation.

I think we should return the signal in units of (induced) charge and not "energy" anymore.
The conversion from charge to energy is done by the electronics which is not part of SSD.
The conversion from the energy of the hit table (input) to number of charge carriers
should happen before the drift is simulated as the self-repulsion needs charge.

@oschulz
Copy link
Member

oschulz commented Jul 23, 2021

Sounds good.

@fhagemann
Copy link
Collaborator Author

I encountered that some keyword arguments (max_tick_distance) always require units which forces the user to install and load Unitful. We should also allow for giving it just values that will be interpreted in SI units.

@fhagemann fhagemann modified the milestones: v1.0.0, v0.7.0 Sep 17, 2021
@fhagemann
Copy link
Collaborator Author

Adding UnitfulRecipes.jl as additional dependency fixes the plotting issue
(we can then just pass values with units to plot and they will automatically be added to the axes labels, quite neat!)

using Plots, Unitful, UnitfulRecipes
plot([1,2]u"m", [1,2]u"m", xlabel = "x", ylabel = "y", legend = false)

Plot1
If you add more stuff with other units, it will keep the units from the first plot command

plot!([1200,1300]u"mm", [1000,1100]u"mm")

Plot2
So, we can just add the internal units to all the objects/values when we plot them.

The question is how we want to deal with external units... Should we allow for a keyword argument in the plot recipe, something like plot(..., length_unit = u"mm")?

@fhagemann
Copy link
Collaborator Author

It should be possible to handle units when it comes to CartesianPoint or CylindricalPoint when passing this to Event, for example...

@lmh91 lmh91 modified the milestones: v0.8.0, v0.9.0 Mar 10, 2022
@fhagemann fhagemann modified the milestones: v0.9.0, v0.10.0 Jun 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
convenience Improve user-friendliness enhancement Improvement of existing features notation Notation and Interface
Projects
None yet
Development

No branches or pull requests

3 participants