-
Notifications
You must be signed in to change notification settings - Fork 22
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
Address possible NaN in ExactSbend & ChrUniformAcc #789
Conversation
Found two examples that illustrate the original problem and this fix. Just trying to decide what works best for testing. |
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.
That looks great, thank you for handling these corner cases!
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 for taking care of this! I just added some meticulous hints to typos.
|
||
In this test, the initial values of :math:`\sigma_x`, :math:`\sigma_y`, :math:`\sigma_t`, :math:`\epsilon_x`, :math:`\epsilon_y`, and :math:`\epsilon_t` must agree with nominal values. | ||
|
||
In addition, the fraction of charge that is lost must agree with the nominal value of 7.7754%, to within a specified tolerance. |
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.
It's a bit unclear where the number 7.7754% is coming from.
# add beam diagnostics | ||
monitor = elements.BeamMonitor("monitor", backend="h5") | ||
|
||
# design the accelerator lattice) |
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.
Little typo: ')' at the end of the line
amrex::ParticleReal const ptf_tot2 = std::pow(ptf_tot,2); | ||
|
||
// check whether particle lies within the domain of map definition | ||
if (pti_tot2 <= 1_prt || ptf_tot2 <= 1_prt) { |
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.
Just a question: Are there coding practices on what to put in an 'if' clause and what in a 'else' clause? I would have naively thought the if(XXX) is testing the good / default case and the 'else:' handling the exception.
Thanks!
bunch_charge_C = 1.0e-9 # used with space charge | ||
npart = 10000 # number of macro particles | ||
|
||
# reference particle |
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.
There are an irregular number of spaces in line 28 and 32 after the '# '
This PR addresses the special case when the square root appearing in the nonlinear map for the ExactSbend element becomes negative -- due, for example, to numerical roundoff.
A similar restriction of the domain of map definition appears for the ChrAcc element, and this is also addressed here. A modified cyclotron example is added with increased energy spread to illustrate the possibility of particle loss.
Fix #599 .
Follow-up: Check for any additional (nonlinear) elements that require further restrictions on the domain of definition to avoid nan's.