-
Notifications
You must be signed in to change notification settings - Fork 191
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
Docs: Thomson Parabola Spectrometer example #5058
base: development
Are you sure you want to change the base?
Conversation
for more information, see https://pre-commit.ci
Examples/Physics_applications/thomson_parabola_spectrometer/plot_detector.py
Fixed
Show fixed
Hide fixed
Examples/Physics_applications/thomson_parabola_spectrometer/plot_detector.py
Fixed
Show fixed
Hide fixed
Examples/Physics_applications/thomson_parabola_spectrometer/plot_detector.py
Fixed
Show fixed
Hide fixed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @aeriforme ! Thanks a lot for this PR! I've left a couple of comments.
In addition we should consider (and I don't have an immediate answer):
- where should we store the figures (our current policy forbids files that are not inputfiles, scripts or README in the example folder)
- if this example should also become a CI test
Examples/Physics_applications/thomson_parabola_spectrometer/plot_detector.py
Outdated
Show resolved
Hide resolved
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks a lot for this new example, @aeriforme .
I left very few comments, but I think that this PR could be merged very soon.
# some parameters from the inputfile | ||
Emax_hydrogen1_1 = 40*MeV | ||
Emax_carbon12_6 = 20*MeV | ||
Emax_carbon12_5 = 20*MeV |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Emax_carbon12_5 = 20*MeV | |
Emax_carbon12_4 = 20*MeV |
Shouldn't this be Emax_carbon12_4
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But in any case it seems that these variables are unused. Maybe you can comment them or delete them
diag1.diag_type = Full | ||
diag1.fields_to_plot = Ex Bx | ||
diag1.format = openpmd | ||
diag1.intervals = 100 | ||
diag1.openpmd_backend = bp | ||
diag1.write_species = 1 | ||
diag1.species = hydrogen1_1 carbon12_6 carbon12_4 | ||
diag1.dump_last_timestep = 1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
diag1
plots the simulation results also at timestep 0, which is what diag0
does. Is this the intended behavior?
Co-authored-by: Luca Fedeli <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @aeriforme ! I have only one tiny comment. Besides of this comment, the PR looks very good.
diag1.diag_type = Full | ||
diag1.fields_to_plot = Ex Bx | ||
diag1.format = openpmd | ||
diag1.intervals = 0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you mean at time zero (0:0
could be more readable, I think) or "at every timestep" (in which case I would suggest 1
)?
for more information, see https://pre-commit.ci
@aeriforme , could you please fix the merge conflicts? |
for more information, see https://pre-commit.ci
This PR adds a new example where different ion species travel through a Thomson Parabola Spectrometer and are collected at a screen.
The example can be found in the
PhysicsApplications/thomson_parabola_spectrometer
folder.