-
Notifications
You must be signed in to change notification settings - Fork 10
Update BeamBeamP, InitialParticleP, TrackingP #127
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
|
I just edited the PR description and removed the template text, so you can add a description specific to the changes in this PR. |
source/parameters/tracking.md
Outdated
| start_loc: "" # [string] Where tracking starts | ||
| end_loc: "" # [string] Where tracking ends |
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.
TrackingP is a parameter group for individual lattice elements. If start_loc and end_loc are global parameters, these parameters should not be in TrackingP. Also if these parameters are particular to one type of simulation and not others, the parameters should be structured within a sub-parameter group.
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.
If TrackingP is for individual lattice elements, don't you need to specify their starting locations w.r.p a global origin?
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 what you mean by "locations w.r.p a global origin" If you are talking about "floor" coordinates then there is FloorP.
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.
David, what parameters in your mind for TrackingP?
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 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.
Should one use that parameter to indicate what tracking method is used in different programs?
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 don't know what you mean by "that parameter". There could be an ImpactT subgroup of TrackingP for example.
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.
Ok, I see your point.
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.
One possibility is to remove your edits of TrackingP since my PR adds documentation.
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.
@DavidSagan OK, I removed my edits of trackingP.
source/parameters/beambeam.md
Outdated
| sigy # The vertical beam size of the opposite beam (default: 0 m). | ||
| xdisp # The horizontal displacement of the opposite beam (default: 0 m). | ||
| ydisp # The vertical displacement of the opposite beam (default: 0 m). | ||
| charge # The charge of the opposite beam (default: 1 for 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.
Better would be
"charge: 0 # [unitless] Charge of particles of the opposite beam."
Also see the syntax used parameters (Example: https://pals-project.readthedocs.io/en/latest/element-parameters.html#bodyshiftp-element-body-orientation-and-position-parameters)
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.
Can you remind me/us, what syntax or style convention do we use to list parameters that don't have a default?
Is this it?
MyParameterGroupP:
param1: 12.3 # [eV] A parameter with default value 12.3 in units of eV.
param2: # [eV] A parameter without default value in units of eV.
param3: # [unitless] A parameter without default value, unitless.Or do we follow a different convention?
Also, how do you write the default specification if it's more intricate than a single value (e.g., with the for protons extra condition in this case)? Like this maybe?
MyParameterGroupP:
param1: 12.3 (for protons) # [eV] A parameter with default value 12.3 (conditional) in units of eV.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.
Okay, so based on #129, this would be
MyParameterGroupP:
param1: 12.3 # [eV] A parameter with default value 12.3 in units of eV.
param2: null # [eV] A parameter without default value in units of eV.
param3: null # [unitless] A parameter without default value, unitless.correct?
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 just put in a PR to clarify that null can be used to signify a lack of a default.
As for the syntax to use when there are multiple defaults, I have not thought about 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.
Perfect.
So, @qianglbl, for now I think we would need to rewrite the lists in this PR following the convention described in my comment above, #127 (comment), namely again
MyParameterGroupP:
param1: 12.3 # [eV] A parameter with default value 12.3 in units of eV.
param2: null # [eV] A parameter without default value in units of eV.
param3: null # [unitless] A parameter without default value, unitless.Please let me know if you want to push a commit with this change or if I shall do 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.
Yes that looks good.
source/parameters/beambeam.md
Outdated
| xdisp # The horizontal displacement of the opposite beam (default: 0 m). | ||
| ydisp # The vertical displacement of the opposite beam (default: 0 m). |
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.
Displacement and orientation can be handled by the BodyShiftP parameter group and do not need to be duplicated.
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.
Agree.
source/parameters/beambeam.md
Outdated
| sigx # The horizontal beam size of the opposite beam (default: 0 m). | ||
| sigy # The vertical beam size of the opposite beam (default: 0 m). |
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.
For hourglass effect will be beta and alpha Twiss parameters.
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.
The sigma is used to calculate beam-beam force kicks. For the hourglass effect, do you mean in beam-beam kick or luminosity?
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.
The hourglass effect is the change in beam size due to changing Twiss beta as a function of longitudinal position.
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 looks like you are thinking about 3D beam-beam instead of 2D beam-beam effects. In that case, one should add sigma_z too.
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.
Correct.
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.
@DavidSagan I updated the beambeam.md following the above discussion and pushed it back.
source/parameters/initialparticle.md
Outdated
| The components of this group are: | ||
| ```{code} yaml | ||
| InitialParticleP: | ||
| Distype: "" # [string] name of initial distribution type |
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.
To be portable, the possible settings of distype (I think better is distribution_type) need to be documented.
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.
Agreed.
source/parameters/beambeam.md
Outdated
| sigx # The horizontal beam size of the opposite beam (default: 0 m). | ||
| sigy # The vertical beam size of the opposite beam (default: 0 m). |
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.
This brings up the question as to whether the standard should use fuller names like sigma_x and sigma_y or shortened names like we have here.
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.
@DavidSagan If we have used fuller names in the other parts of the standard, we should probably use the fuller names to be consistent.
source/parameters/tracking.md
Outdated
| The components of this group are: | ||
| ```{code} yaml | ||
| ReferenceP: | ||
| tracking_type: "" # [string] type of tracking |
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.
This is in contrast to https://github.com/pals-project/pals/pull/119/files#diff-b31d07bf5b91fc4d9071400bebdba5dfa63f852f178f80b23d1612e1892b05f1 where the type of tracking can be set on a per-program basis. This raises the question as to whether a overall tracking method setting is useful.
source/element-kinds.md
Outdated
|
|
||
| An RFCavity element is an RF cavity without acceleration generally used in a storage ring. The main difference | ||
| between an rfcavity and an lcavity is that, unlike an lcavity, the reference energy through | ||
| an rfcavity is constant. |
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.
Note: This is how things are done in the BMAD manual, but I'm not sure we should do this in PALS. Note that David didn't add a separate LCavity element or parameters, so I suspect he intended to combine the functionality into one element kind.
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.
This should be discussed. The only difference in Bmad between RFCavity and Lcavity is the reference energy change.
| Element for simulating colliding beams. | ||
|
|
||
| Under Construction... | ||
| A BeamBeam element defines the parameters of a oppositely moving "strong" beam that generates electromagnetic fields at the interaction point. This strong beam is assumed to have a three-dimensional (3D) Gaussian density distribution. |
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 we really want to have the assumption that the strong beam is Gaussian. If a program does not make this assumption then what kind of lattice element should it use?
source/element-kinds.md
Outdated
| Global coordinate system fiducial point. | ||
| A Fiducial element is used to fix the position and orientation of the reference orbit within the global | ||
| coordinate system at the location of the fiducial element. A fiducial element will affect the global | ||
| floor coordinates (§16.2) of elements both upstream and downstream of the fiducial element. |
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.
Section numbers should not be hard coded.
source/element-kinds.md
Outdated
| It uses the hkick and vkick parameters to deflect the beam in | ||
| horizontal and vertical directions. |
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 we really want different names for kicks as opposed to zeroth order multipoles?
Co-authored-by: Chad Mitchell <[email protected]>
| InitialParticleP: | ||
| distribution_type: "" # [string] name of initial distribution type | ||
| sigma_x: null # RMS x = sqrt(<x^2>) | ||
| sigma_px: null # RMS px = sqrt(<px^2>) | ||
| mu_xpx: null # <xpx>/(sigma_x*sigma_px) where <> denotes average over distribution | ||
| x_off: null # <x> | ||
| px_off: null # <px> | ||
| sigma_y: null # RMS y = sqrt(<y^2>) | ||
| sigma_py: null # RMS py = sqrt(<py^2>) | ||
| y_off: null # <y> | ||
| py_off: null # <py> | ||
| mu_ypy: null # <ypy>/(sigma_y*sigma_py) | ||
| sigma_z: null # RMS z = sqrt(<z^2>) | ||
| sigma_pz: null # RMS pz = sqrt(<pz^2>) | ||
| mu_zpz: null # <zpz>/(sigma_z*sigma_pz) | ||
| z_off: null # <z> | ||
| pz_off: null # <pz> | ||
| ``` |
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 21 moments so setting any should be available. How about something like sigma_xpy or sigma_14 for <x*py>. Also something like <xpx>/(sigma_x*sigma_px) will be singular if sigma_x or sigma_px are zero and these are redundant if sigma_14 notation is used.
source/parameters/initialparticle.md
Outdated
| InitialParticleP: | ||
| distribution_type: "" # [string] name of initial distribution type | ||
| x_off: null # <x>, <> denotes average over distribution | ||
| px_off: null # <px> | ||
| y_off: null # <y> | ||
| py_off: null # <py> | ||
| z_off: null # <z> | ||
| pz_off: null # <pz> | ||
| sigma_x: null # RMS x = sqrt(<x^2>) | ||
| sigma_px: null # RMS px = sqrt(<px^2>) | ||
| sigma_y: null # RMS y = sqrt(<y^2>) | ||
| sigma_py: null # RMS py = sqrt(<py^2>) | ||
| sigma_z: null # RMS z = sqrt(<z^2>) | ||
| sigma_pz: null # RMS pz = sqrt(<pz^2>) | ||
| mu_xpx: null # <xpx> | ||
| mu_xy: null # <xy> | ||
| mu_xpy: null # <xpy> | ||
| mu_xz: null # <xz> | ||
| mu_xpz: null # <xpz> | ||
| mu_pxy: null # <pxy> | ||
| mu_pxpy: null # <pxpy> | ||
| mu_pxz: null # <pxz> | ||
| mu_pxpz: null # <pxpz> | ||
| mu_ypy: null # <ypy> | ||
| mu_yz: null # <yz> | ||
| mu_ypz: null # <ypz> | ||
| mu_pyz: null # <pyz> | ||
| mu_pypz: null # <pypz> | ||
| mu_zpz: null # <zpz> |
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 suggest a uniform naming for the sigma matrix. EG: sigma_xx = <xx>, etc. Or can use covar in place of sigma. Another naming convention, if we want to contemplate PALS supporting differing phase space coordinate systems, is to use numbers for subscripts. EG: sigma_12 = <x*py>.
Co-authored-by: Chad Mitchell <[email protected]>
Co-authored-by: Chad Mitchell <[email protected]>
| t_offset: 0.1e-8 | ||
| scale_multipoles: = F | ||
| b1: = 0.27 | ||
| amplitude_vs_time: = {(-1.2e-6, 0.02), ... } |
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.
This is derived from Bmad and I don't think this is a good fit for PALS. My feeling is that using an expression to define the AC waveform as a function of time is cleaner. In any case, all new parameters need to be well defined.
| - [**ReferenceP**](#s:ref.params): Reference parameters. | ||
| - [**TrackingP**](#s:tracking.params): Tracking parameters. | ||
| The length of this element is considered to be zero so if `length` is specified, it must be zero. |
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.
Why is this taken out?
No description provided.