Skip to content
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

Fix atBunchLength #585

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Fix atBunchLength #585

wants to merge 3 commits into from

Conversation

carmignani
Copy link
Member

atBunchLength now checks if the ring is 4D or 6D. If it is 4D, the needed parameters are computed with ringpara, if it is 6D they are computed with atx.
The function blgrowth has been separated from BunchLength, so it can be used by both BunchLength and atBunchLength.

U0 = ringdata.eloss;
sigdelta = ringdata.espread;
bl0 = ringdata.blength;
BL = bl0 * blgrowth(Ib,Zn,Vrf,U0,E0,h,alpha,sigdelta);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are some possible optimizations here:
alpha -> mcf(disable_6d(ring))
energy, vrf, h -> atGetRingProperties
sigdelta, bl0 -> ohmi_envelope
U0 -> atgetU0(ring, 'method', 'tracking')

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On top of @swhite2401 proposal:

is6d and alpha are also available through atGetRingProperties. Asking for is6d, Vrf, E0, h, alpha and circ in one call (before the if test) minimises the number of passes through the lattice. Be careful to use the "single cell" attributes: cell_rf_voltage, cell_harmnumber, cell_length.

You could also get both bl0 and sigdelta from ringpara and completely forget BunchLength.

@carmignani
Copy link
Member Author

carmignani commented Mar 27, 2023

alpha -> mcf(disable_6d(ring))

I was thinking to use mcf for alpha, but then it would be the on-energy value and I think here we need the off-energy one in case of off-energy lattice.
I assumed that atx was giving the off-energy alpha, but apparently it is still the on-energy one. Is it a bug?

@lfarv
Copy link
Contributor

lfarv commented Mar 27, 2023

@carmignani : atx calls mcf, and mcf should return the correct off-energy value… I'll check

@carmignani
Copy link
Member Author

@lfarv : I tried atx with a 6D lattice on-energy and with Delta frequency of 300 Hz and alpha returned by atx is exactly the same.

@lfarv
Copy link
Contributor

lfarv commented Mar 28, 2023

@carmignani : I confirm, there is a bug in atx, the dp argument provided to mcf is wrong (taken from the 4d orbit…). Things get more complicated…

@lfarv
Copy link
Contributor

lfarv commented Mar 28, 2023

@carmignani, @swhite2401:

The problem, for a given 6D lattice, is to derive its off-momentum dp so that it can be given to mcf. The solution is: get df = (RF frequency - nominal frequency), from df derive the path lengthening dct: dct = β.c.df/f2, then compute the 4d closed obit with this df, this will give dp in the 5th coordinate. But I do not know where to put this: in atBunchLength ? mcf ? in atx ?

More: if the 6d lattice results from a frequency_control decorator (or equivalent in Matlab), the dp was maybe specified by the user to generate the RF frequency, and maybe not. We should avoid double computation…

Up top now, (in linopt6 I think) this was approximated by simply computing the 6d closed orbit and taking out the dp at the entrance of the lattice. This is approximate…

@swhite2401
Copy link
Contributor

@lfarv, I would implement the correction at the lowest level such if similar issue exist in other function they are fixed as well.
Then we have to distinguish 2 cases as you describe. For 4D lattice, the present method is fine. For 6D, as you said this is approximate but do you have other ideas?
Computing the average dp could be less approximate but is also less efficient, another alternative is to derive the integral D/rho, this can be done in a similar fashion as the radiation integrals... but is also subject to errors.

@lfarv
Copy link
Contributor

lfarv commented Mar 29, 2023

@swhite2401:

For 6D, as you said this is approximate but do you have other ideas?

  • The approximate method is to take the dp at origin of the 6d closed orbit (instead of the average). But it's correct for differential dp (subtracting 2 closed orbits at different frequencies), as it is done in linopt6,
  • The exact method is the one I described above. It's not so expensive: deriving a 4d lattice (anyway necessary for mcf), and 4d closed orbit computation. Probably not worse than computing the integral, and works in all conditions (vertical dispersion, coupling…). It's just the inverse of what is done in frequency_control to get the needed frequency.

I would implement the correction at the lowest level

This means all functions needing the dp value for any use… I have no exhaustive list, and it could result in multiple computations… But may be doing it in mcf will solve most of the needs.

@swhite2401
Copy link
Contributor

Ok understood, it would be nice to have this list of functions, and if they are used in core AT functions... double computations could be acceptable in ringparam that does need to be particularly fast for example but not in find_orbit

@lfarv
Copy link
Contributor

lfarv commented Mar 29, 2023

it would be nice to have this list of functions

Yes, of course… But I even ignored the existence of atBunchLength… And I don't see how we could do an automatic search. Hence my proposal to start with mcf, which could fix the functions we are aware of at the moment: ringpara, atsummary, atx, atBunchLength.

mcf would accept 4D ot 6D lattices, with dp, dct and df arguments. At the cost of one conversion to 4D and 2 or 3 closed orbit computations (2 are anyway already necessary).

@lfarv lfarv mentioned this pull request Apr 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants