-
Notifications
You must be signed in to change notification settings - Fork 14
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
FieldQuadrupole element based on FieldElement #147
Conversation
Quadrupole element based on the FieldElement class for tracking a bunch through a quadrupole without approximations
- move to the class definition for consistency - rename "plasma lens" to "quadrupole" in the description.
The | operator for types was introduced only in Python 3.10.
Hi @agolovanov , thanks a lot for implementing this, and for taking care of the issues in the tests. Overall it looks good to me. I would however suggest changing the sign of the field, so that a positive value is focusing for electrons on the x plane. This would make it more consistent with the existing |
Regaring the docstring of the |
@AngelFP I used the other sign because that's how it was written in the formula in Wikipedia. You are right that it's better for consistency with already existing code to flip the sign, so I've changed it. I've also added a new test comparing FieldQuadrupole with the existing Quadrupole. Thank you for implementing the automatic |
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.
Looks great! Thanks for making the changes and adding a new test :D
Quadrupole element based on the FieldElement class for tracking a bunch through a quadrupole without approximations.
My current implementation is uniform in longitudinal direction field, without Bz, without the skew component (so orientation only along x or y depending on the sign of
foc_strength
), and without automatic estimation of the needed timestep. All these missing things are possible improvements to the element.I am not completely satisfied with the class name
FieldQuadrupole
(asQuadrupole
is already in use), but couldn't come with anything nicer.I tested it against my own calculation of a thin quadrupole (without Wake-T), but it can't be used as an automated test. Should I design a test against the existing TM quadrupole element for some monoenergetic bunch?
Also, I noticed that docstrings for classes are given after the class. Is this a convention in this project? PEP 257 recommends putting docstrings for the
__init__
method instead: The class constructor should be documented in the docstring for its__init__
method.I also didn't check how to add the new element to the API reference in generated documentation or whether it is done automatically.