-
Notifications
You must be signed in to change notification settings - Fork 10
Conventions Updates & Matching Syntax #135
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
base: main
Are you sure you want to change the base?
Conversation
| Constants defined by PALS: | ||
| ```{code} yaml | ||
| c_light # [ m/sec] Speed of light | ||
| h_planck # [eV*sec] Planck's constant | ||
| h_bar_planck # [eV*sec] Reduced Planck's constant | ||
| r_e # [m] Classical electron radius | ||
| r_p # [m] Classical proton radius | ||
| e_charge # [Coul] Elementary charge | ||
| mu_0_vac # [eV*sec^2/m] Vacuum permeability | ||
| eps_0_vac # [1/eV*m] Permittivity of free space | ||
| classical_radius_factor # [m*eV] Classical Radius Factor: 1/(4 pi epsilon_0 c^2) | ||
| fine_structure_const # [-] Fine structure constant | ||
| n_avogadro # [-] Avogadro's constant | ||
| ``` | ||
| The `classical_radius_factor` is a useful number when converting a formula that involve the classical | ||
| electron or proton radius to a formula for something other than an electron or proton. |
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.
We had this debate a few times for some of the BLAST codes in the past and we may want to have it here too.
I think many scientists are familiar, and used to, SciPy's naming conventions for constants, defined in https://docs.scipy.org/doc/scipy/reference/constants.html.
Maybe it could be beneficial to follow the same naming conventions (e.g., epsilon_0 instead of eps_0_vac, hbar instead of h_bar_planck, etc.) as much as possible.
I would personally be in favor of this, but I suggest that we could vote on it.
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.
In case that's interesting, I think (some of) the naming conventions that Xsuite adopted can be found in the following files:
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 is nothing in the Xsuite conventions that I would use for PALS. Note: with PALS there are functions for mass. For example: mass_of(electron).
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.
I'm not sure how many people are familiar with the SciPy constants. In any case, I am not in favor of using short names like c and h for constants since this could be confusing. But we can have a vote on this if someone wants it. @EZoni Do you want to make specific suggestions now? Alternatively, we can use what is here as a starting point and future PRs can be made when changes/additions are wanted. What do you think?
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.
I'm fine with not blocking the PR per se, but I will anyways open a few targeted GitHub polls for the ones that I would like to suggest.
|
If you want, I could open a Poll to let people vote on the following options:
I can also remove option 1. since we already discarded it in the live poll. Feel free to edit this comment if you want to refine the way we phrase the options. |
| sqrt(x) # Square Root | ||
| log(x) # Logarithm | ||
| exp(x) # Exponential | ||
| sin(x), cos(x) # Sine and cosine | ||
| tan(x), cot(x) # Tangent and cotangent | ||
| sinc(x) # Sin(x)/x Function | ||
| asin(x), acos(x) # Arc sine and Arc cosine | ||
| atan(x) # Arc tangent | ||
| atan2(y, x) # Arc tangent of y/x | ||
| sinh(x), cosh(x) # Hyperbolic sine and cosine | ||
| tanh(x), coth(x) # Hyperbolic tangent and cotangent | ||
| asinh(x), acosh(x) # Hyperbolic arc sine and Arc cosine | ||
| atanh(x), acoth(x) # Hyperbolic arc tangent and cotangent | ||
| abs(x) # Absolute Value | ||
| factorial(n) # Factorial | ||
| ran() # Random number between 0 and 1 | ||
| ran_gauss() # Gaussian distributed random number | ||
| ran_gauss(sig_cut) # Gaussian distributed random number | ||
| int(x) # Nearest integer with magnitude less then x | ||
| nint(x) # Nearest integer to x | ||
| sign(x) # 1 if x positive, -1 if negative, 0 if zero | ||
| floor(x) # Nearest integer less than x | ||
| ceiling(x) # Nearest integer greater than x | ||
| modulo(a, p) # a - floor(a/p) * p. Will be in range [0, p). | ||
| mass_of(A) # Mass of particle A | ||
| charge_of(A) # Charge, in units of the elementary charge, of particle A | ||
| anomalous_moment_of(A) # Anomalous magnetic moment of particle A | ||
| ``` |
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.
Similar comment as for the constants above, we could try to match some of the NumPy conventions, e.g., ceil instead of ceiling (which would incidentally also match C++'s std::ceil), random instead of ran, etc.
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.
Personally I don't like ceil. Changing to random is fine. Do you want to discuss changes now or merge and discuss changes in future PRs?
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.
Let's already update random. Also let us document that we mean a uniform distribution between 0 and 1 for random()
If we go to mixed case I would remove the underscores and capitalize the first word. EG: |
|
Consensus in the meeting of Jan 7th: |
Conventions modifications moved from PR #119.