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

TWISS drift with EXACT flag has errors in the time variable #1123

Merged

Conversation

jsberg-bnl
Copy link

The code for the exact drift for twiss miscalculates the time components of the linear and second order maps (magnitude, not sign). Here's an example demonstrating it: 220614a13.txt; all the value commands should output something zeroish. Note for the example to make sense, you need to have applied #1121 (the fix for the sign error for the time variable in linear and higher order maps in PTC).

@rdemaria
Copy link
Contributor

Great! Could you add the file 220614a13.txt as additional twiss-test?

@rdemaria rdemaria added the bug the behaviour is not documented and should be fixed in the code label Jun 20, 2022
@jsberg-bnl
Copy link
Author

I have the test all set up; as written it depends on #1121 having been applied as well. Do you want me to add it to the pull request as-is, comment out lines in the the .cfg files to only check the twiss and sectormap output for now (so that it's not dependent on the ptc changes in #1121), or create a separate pull request for the test?

@rdemaria
Copy link
Contributor

I merged #1121 , so you can add the test. Two more things.

  1. @ldeniau remarked that in the exact_expansion branch, the line:

    ek(5) = dl*dtbyds

is missing! This is needed to introduce energy error in presence of cavities. Can you add or remove the line outside the if statement?

  1. can you add a line in Changes.md.

Thanks again!

@jsberg-bnl
Copy link
Author

I've added the test case to the pull request.
I've also put in the code to do the equivalent of ek(5) = dl*dtbyds. If I understand correctly, this is to handle the nonzero deltap case. I think what I put there is consistent with what has been done elsewhere, but I am not 100% sure of how nonzero deltap is intended to be handled. As I understand it, a correction is made to the time variable with nonzero deltap so that the time variable for one period comes back to zero, at least nearly so. The contribution at each element is based on dt/d(pt) computed from the one turn linear map, the nominal element length, and the nominal circumference. PTC handles nonzero deltap differently, it seems to simply set pt to the sum of deltap (converted to pt) and any supplied pt, keeping the time offset relative to the reference time (similar to what you would have with twiss with zero deltap and a nonzero pt).

@rdemaria
Copy link
Contributor

Below there is some explanation for deltap (a parameter that introduces an energy error and redefine momenta) and pt (the coordinate). Indeed, PTC, as far as I know, does not have the same mechanism to introduce energy error. I think it can be done using total_time=True and changing the RF frequency, but I have never tried.

image

@rdemaria
Copy link
Contributor

In PTC we could introduce a dtbyds like in TWISS to obtain the same functionality; however, we would need to keep in mind that the momenta will differ from those of MAD-X, that are scaled by Ps. In TWISS the scaling by Ps is essential to keep pt small, given that transfer map are correct in pt to first order (beside now the drift in the exact option). If we equipped TWISS with correct transfer maps (which we will very likely be forced to do to handle FCC lattices) we could abandon the Ps normalization in favour of P0 like PTC. Still, even in this context, the dtbyds is an useful feature to keep to study energy errors.

@jsberg-bnl
Copy link
Author

Thanks @rdemaria for that info. It would be helpful to get the explanation correct in the MAD-X manual, what's there appears to be incomplete and not entirely consistent. Given that, the code for exact drifts should be handling everything correctly (I did some checks on a FODO cell with dipoles with and without exact drifts).
One thing to be aware of is that there are lattices for which the t variable, even on the design energy, will not be zero after one turn: i.e., the length of the closed orbit is not equal to the sum of the element lengths. This occurs, for example, when intentionally going off-axis through magnets. As you said, this can be handled in PTC using total_time and adjusting the RF frequency. Another alternative would be to add an element (possibly just an added parameter to translation) which shifts the reference time (thereby making a corresponding shift in t); the shift would also contribute to the RF frequency calculation when based on the harmonic number.

@tpersson
Copy link
Contributor

tpersson commented Aug 1, 2022

This is very nice! Indeed there was an error in the time variable for the maps. I also discovered this but then the LHC commissioning got in the way. For me this is fine to merge since it does the same thing but with the correct time!

@rdemaria rdemaria merged commit 674e770 into MethodicalAcceleratorDesign:master Aug 15, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug the behaviour is not documented and should be fixed in the code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants