-
Notifications
You must be signed in to change notification settings - Fork 32
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
Remove the dependency of RFCavityPass on harmonic number #319
Conversation
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 for me.
Just one minor comment: T0 is derived from a hard coded speed of light in at.c, this could potentially introduce minor numerical errors since we are using a different source to derive the "nominal" frequency in python. These could become visible for long term tracking.
This is certainly not a big issue, but I wonder whether there is a possibility to make things consistent.
I think that this also requires an update of/change in ring_parameters.py: voltage = ring.voltage -> ring.rf_voltage at/pyat/at/physics/ring_parameters.py Line 103 in eb3fe84
|
We could put the |
As you probably know, the core beam dynamics C code for AT, is a manual translation of the "numerical engine" for the beam dynamics Pascal library, aka Tracy-2, that I implemented 1990; having been tasked with providing an on-line model to guide the ALS commissioning. Hence, as a side effect, I could also calibrate the model; vs. a modern synchrotron light source. Instead, for the SLS commissioning M. Böge & J. Chrin made a machine translation of it (with p2c) & implemented it as a client/server architecture: M. Böge, J. Chrin “A Corba Based Client-Server Model for Beam Dynamics Applications at the SLS” ICALEPS 1999 Bottom line, clearly, the harmonic number is a property of the RF Cavity object/structure/class. |
On the principle you are right. However in the particular case of speed of light, since it's a fundamental absolute value (no uncertainty, no rounding, no error), it does not matter. The values are exactly the same (you can try and learn the 9 digits, there are no more…). I keep in mind to look at a way to share constants. |
@jbengtsson: Thanks for pointing out, it's corrected on the master branch. |
Done. |
@carmignani, @jbengtsson, @swhite2401: Does A more urgent question is: should this passmethod be the default one for cavities ? I'm in favour, since as @carmignani pointe out, Any comment on that? |
To me it's a "no brainer" (harmonic number); but my basic training is MsEE.
I.e., if not ("obvious") – for a 2nd (hands-on) opinion – ask any of the
local RF engineers.
Remarks:
• The reason:
M. Böge, J. Chrin *A Corba Based Client-Server Model for Beam Dynamics
Applications at the SLS* ICALEPS 1999
could machine translate my ca 10,000 of Pascal library/code from 1990. Was
because I'd taken care to structure/layer/architecture the *beam dynamics
Pascal module* (akin to C++ object) & *software library* (Because
beforehand, I'd been working as a TA in software engineering; i.e., how to
implement scalable & reusable code, compiler design & implementation, etc).
• Clearly, the RF cavity has one (symplectic) integrator. But, as in the
control room, the RF voltage can e.g. be set to zero; vs. ripping it out &
putting it back in.
• The internal Tracy-2 beam dynamics model is transparent & simple; i.e.,
full 6D phase-space.
Hence, this was automatically propagated by the machine translation
by Michael & Jan (of the entire beam dynamics model)
However, this was apparently missed for the manual translation of ditto for
AT (for which only a subset was ported).
In other words, already 1990, I was e.g. computing the vertical emittance
and, hence, the SLS model server as well; i.e., the linear invariants for
6D phase-space (a la A. Chao, vs. the *beam envelope formalism* from 1996).
…On Tue, Oct 19, 2021 at 3:11 PM Laurent Farvacque ***@***.***> wrote:
@carmignani <https://github.com/carmignani>, @jbengtsson
<https://github.com/jbengtsson>: Does HarmonicNumber belongs to the
cavity element ? It looks there is a debate about that. For the time being,
I propose to follow @carmignani <https://github.com/carmignani>: keep it
as an optional attribute. The point here is that wherever it is, it is not
necessary for tracking, so we ignore it.
A more urgent question is: should this passmethod be the default one for
cavities ? I'm in favour, since as @carmignani
<https://github.com/carmignani> pointe out, CavityPass is wrong.
Any comment on that?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#319 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/ACLNK4P6WXOOAQM4JUVD753UHVVA3ANCNFSM5GHDA7JQ>
.
Triage notifications on the go with GitHub Mobile for iOS
<https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675>
or Android
<https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.
|
In RFCavityPass the harmonic number is needed in the tracking. The proposed modification is fine, because you compute h from the Frequency and the circumference, but I would keep it as optional field. In case of very off-energy simulations, the computation could fail by one unit in the harmonic cavities. |
As I stated initially, in the original Tracy-2 ALS on-line model (1990),
the harmonic number is a *property* of the RF cavity (not something being
computed; i.e., nonsense to me).
In other words, it's one of the key RF engineering
parameters/considerations for a conceptual design, etc.
The only reason its needed for the RF cavity integrator, is because – in
the case of computation of total (vs. differential) pathlength for the 5th
phase-space coordinate – for each turn its value is reduced by:
ct -> ct - harm_no / f_rf * c
i.e., the design circumference C.
Or else, soon enough multi-turn tracking would lead to numerical overflow
(since ct is increased by roughly C for each turn).
Besides, it still is. I.e., I never had to change the core/internal of my
numerical engine for Tracy-2 (utilised for: ALS, SLAC B-Factory,
SLS, SOLEIL, etc.).
…On Tue, Oct 19, 2021 at 4:39 PM Nicola Carmignani ***@***.***> wrote:
@carmignani <https://github.com/carmignani>, @jbengtsson
<https://github.com/jbengtsson>, @swhite2401
<https://github.com/swhite2401>: Does HarmonicNumber belongs to the
cavity element ? It looks there is a debate about that. For the time being,
I propose to follow @carmignani <https://github.com/carmignani>: keep it
as an optional attribute. The point here is that wherever it is, it is not
necessary for tracking, so we ignore it.
In RFCavityPass the harmonic number is needed in the tracking. The
proposed modification is fine, because you compute h from the Frequency and
the circumference, but I would keep it as optional field. In case of very
off-energy simulations, the computation could fail by one unit in the
harmonic cavities.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#319 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/ACLNK4L6NDCRFHBA7BSFEI3UHV7KVANCNFSM5GHDA7JQ>
.
Triage notifications on the go with GitHub Mobile for iOS
<https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675>
or Android
<https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.
|
No, it's not necessary ! In the formula quoted by @jbengtsson
harm_no can be any integer, it does not change the result of the sine function. It's only there to avoid numerical overflow. I do not compute the harmonic number from the frequency, I compute the integer which minimises the argument of the sine function.
OK, so we can call this computed integer with another name, no problem. What @carmignani mentions is valid for |
Possibly, but you can also consider it as a lattice property: number of buckets along the circumference. That defines the main RF system. It has much less meaning for an harmonic cavity. |
It's unclear to me why the orbit finder would need such tweaks; since it
only tracks one turn.
As I stated, the only reason I subtract *C* for each turn is for tracking
with absolute/total pathlength; or else s -> n*C. Which for longterm
tracking eventually leads to FP overflow.
…On Tue, Oct 19, 2021 at 9:33 PM Laurent Farvacque ***@***.***> wrote:
In RFCavityPass the harmonic number is needed in the tracking. The
proposed modification is fine, because you compute h from the Frequency and
the circumference, but I would keep it as optional field. In case of very
off-energy simulations, the computation could fail by one unit in the
harmonic cavities.
No, it's not necessary ! In the formula quoted by @jbengtsson
<https://github.com/jbengtsson>
ct -> ct - harm_no / f_rf * c
harm_no can be any integer, it does not change the result of the sine
function. It's only there to avoid numerical overflow. I do not compute the
harmonic number from the frequency, I computed the integer which minimises
the argument of the sine function.
the harmonic number is a *property* of the RF cavity (not something being
computed; i.e., nonsense to me)
OK, so we can call this computed integer with another name, no problem.
What @carmignani <https://github.com/carmignani> mentions is valid for
find_orbit6: there it's true, the path lengthening is computed with
respect to the the nearest harmonic number, which for very large frequency
deviations may be different form the real harmonic number.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#319 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/ACLNK4OT3D3JKKIH6TO6XSLUHXBZXANCNFSM5GHDA7JQ>
.
Triage notifications on the go with GitHub Mobile for iOS
<https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675>
or Android
<https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.
|
True.
However, for the original implementation (1990), for simplicity for the
beam line (Pascal) structure/record, I took a LEGO approach (something that
I grew up with); which hasn't changed to date.
In other words, each element has only local properties/attributes, which I
then assemble into a ring; or beam line, collider, re-circulator, etc., for
that matter.
I.e., the internal numerical engine makes no assumption on whether its
periodic or single pass system.
Hence, e.g. the *orbit finder* is on the *user level*, i.e., depends
on the *use
case*.
Clearly, the harmonic number is given by the circumference & number of
buckets; i.e., in the case of a periodic system.
And the former is a number that I can obtain from e.g. the RF engineer; for
a local description of their subsystem/component/LEGO piece.
And, if there wasn't an FP overflow issue with longterm tracking, the RF
cavity object only has local parameters: length, voltage, frequency, and
phase; the latter defined relative to the design orbit/co-moving frame.
…On Tue, Oct 19, 2021 at 9:43 PM Laurent Farvacque ***@***.***> wrote:
the harmonic number is a *property* of the RF cavity (not something being
computed; i.e., nonsense to me).
In other words, it's one of the key RF engineering
parameters/considerations for a conceptual design, etc.
Possibly, but you can also consider it as a lattice property: number of
buckets along the circumference. That defines the main RF system. It has
much less meaning for an harmonic cavity.
Anyway, we can keep it in the cavity class, anyone is free to any
attribute to an element.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#319 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/ACLNK4IRDK7RHRQABSSOM2DUHXC7HANCNFSM5GHDA7JQ>
.
Triage notifications on the go with GitHub Mobile for iOS
<https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675>
or Android
<https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.
|
The implementation proposed in this PR is perfectly fine and can be merged in its present state. This Concerning the harmonic number, it should noted that the historical AT The cavity object is defined by its voltage, frequency, timelag (phase), and length as mentioned above, the harmonic number is derived from the frequency and the length of the machine: the same cavity object can be used in different lattices with difference harmonic numbers. What @lfarv proposes, to attach the harmonic number to the lattice, is therefore appropriate. As far as I am concerned this PR represents an significant improvement and I renew my approval. |
Dear All, Nice conclusion & summary. Bottom line, what's essential (I think) is to (strategically) make a clear Best regards. |
Yes, that is true. Then you are right and we can remove the harmonic number. |
Going on with removing harmonic number where it is not necessary, we now remove it from
RFCavityPass
.