-
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
Simple Ring modification to solve energy loss computation bug #710
Conversation
…ples, changed radiation to longdict in element, modified list of disable_6d passmethods in get_energy_loss and modified fast ring
Hello @lcarver and @swhite2401 The idea is nice but I think that the implementation is wrong. More exactly, In the general 6D case; the 6x6 one-turn transfer matrix Mrad can be expressed using the A matrix as: Mrad = A . R . Λ . A-1 with R block diagonal rotation matrix (phase advance) and Λ diagonal damping matrix. so we have: Mrad = (A . R . A-1) . (A . Λ . A-1) Mrad = Mnorad . DAMP The damping matrix DAMP is A . Λ . A-1 ! Λ is similar to what you wrote in
In conclusion, this option (once corrected) costs an additional matrix multiplication. |
@lfarv, this algorithm is implemented in many well know codes, is well benchmarked and gives the correct result in term of damping time and equilibrium emittance. This is exactly what we are looking in multi-particle simulation where the full ring is represented by a matrix. We could go for your solution but then the algorithm becomes more complicated since you have to derive the 6x6 damping matrix, providing the huge approximation we are already making in modeling the ring I do not see much interest in this. |
@swhite2401 : I have no doubt that you get the correct damping times and emittances, but for sure the transverse motion is wrong. The damping is applied as if the betas were 1.0. You maybe don't care about transverse optics: you can run multi particle tracking were the turn is defined by simply a phase advance, no need for beta functions, the one turn map is a simple rotation, and then your formula is correct. You just define tunes, chromaticities, and that's it. But if for some reason you want to introduce betas and alphas, it's different. I'm 100% sure the formulas above are correct, so anything else is wrong. But I understand the need for speed, so I had a look numerically: in my notations, Λ = diag([exp(-1/τx), exp(-1/τx), exp(-1/τy), ...]) with τ in turns. Λ For a sample ring ( DAMP ≈ diag([exp(-1/τx), exp(-1/τx), exp(-1/τy), ...]) DAMP ≈ diag([(1-1/τx), (1-1/τx), (1-1/τy), (1-1/τy), (1-1/τz), (1-1/τz)]) for τ >> 1 Compared to what is in
|
Coming back to more practical problems: I agree that a class SimpleRadiation(_DictLongtMotion, Radiative, Element): in that order, so that |
I modified Added Assuming the following formula to move from real to normalised coordinates:
The particles are converted, damping applied, then reverted. U0 is applied at the end after all changes. |
Below is a test file for your convenience:
|
Ready for re-review. |
Fixed dispersion problem. Should be ok now. Ready for review. |
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 now correct (at least I think so), but can be simplified.
…ild attributes, add exponent instead of linear
all changes implemented, but one outstanding question to be resolved (I made a comment in the code) |
@lcarver : I just found out that the linear matrix is missing the dispersion. One must add: M04 = (1.0 - M00) * dispx - M01 * dispxp
M14 = -M10 * dispx + (1.0 - M11) * dispxp
M24 = (1.0 - M22) * dispy - M23 * dispyp
M34 = -M32 * dispy + (1.0 - M33) * dispyp |
Well spotted. I added your modifications to the M66. |
If this was a test, I admit that I failed… What is this question ? |
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.
Provided your test still succeeds, that's OK for me.
This is proposed as a solution to #694 .
Old simple ring contained:
ring = [RF Cavity, M66 no rad, Nonlinear element, SimpleQuantDiff]
where
SimpleQuantDiff
handled all radiation effects (damping, diffusion and U0)This setup was problematic due to the fact that
get_energy_loss
could not compute theU0
without the damping and diffusion, which had some other knock-on problems.Now simple ring contains:
ring = [RF Cavity, M66 no rad, SimpleRadiation, Nonlinear element, SimpleQuantDiff]
where
SimpleRadiation
is a new PassMethod that only performs U0 and radiation damping andSimpleQuantDiff
has been modified to only act as the diffusion source for emittance.One possible problem that I want to draw to your attention, is that now we have 2 diffusion elements (QuantumDiffusion and SimpleQuantDiff) and I wonder if it is now more convenient to create a
Diffusion
class/group (not sure of the correct noun) and then each element has the propertyis_diffusion
to enable cleaner code (for example you can see inget_energy_loss
I have to mention both elements specifically.If this is merged, then the structure can become the basis for the
fast_ring
reorganisation, which will be done in a separate PR.