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

Exact pass #574

Closed
wants to merge 43 commits into from
Closed

Exact pass #574

wants to merge 43 commits into from

Conversation

simoneliuzzo
Copy link
Contributor

I open this draft pull request for the exact_pass branch in order to list the issues I find while using the branch.

The exact_pass branch implements 2 additional PassMethods ExactDriftPass and ExactMultipolePass. Those are necessary for drift and multipoles with strong variations of optics, such as in final focus lattices for colliders.

@simoneliuzzo
Copy link
Contributor Author

atradon/atradoff not working in this branch. *Rad equivalents are needed before merge.

@simoneliuzzo
Copy link
Contributor Author

simoneliuzzo commented Mar 6, 2023

atx emittance computation fails. Likely ohmi envelope internal tracking must also become aware of the new integrators

b =

struct with fields:

          ll: 9.1664e+04
       alpha: 9.1210e-06
   fractunes: [0.2993 0.3020]
   fulltunes: [362.2993 322.3020]
         nuh: 362.2993
         nuv: 322.3020
chromaticity: [0.3579 0.4701]
 dampingtime: [1.0566e+05 3.6610e+04 1.3672e+05]
    dampingJ: [0.8585 2.4779 0.6635]
     espread: 0
     blength: 0
modemittance: [NaN NaN NaN]
      energy: 1.8250e+11
          fs: 588.9771
       eloss: 0
synchrophase: 0
  momcompact: 9.1210e-06

@simoneliuzzo
Copy link
Contributor Author

missing ExactBendPass and ExactBendRadPass

@simoneliuzzo
Copy link
Contributor Author

missing tests of each passmethod.
For example comparison of results for tracking in an extremely long drift?

@swhite2401
Copy link
Contributor

Hello @simoneliuzzo , sorry I haven't been very good at providing these benchmarking test, I will get on to it.
I can put the result of this benchmarking in a 'hardcoded' unit test.

@lfarv
Copy link
Contributor

lfarv commented Mar 8, 2023

@simoneliuzzo : summary of the answers to your questions:

atradon/atradoff not working in this branch. *Rad equivalents are needed before merge.

exactMultipoleRadPass looks easy to implement. Once this is done, I would suggest merging ExactDrift and ExactMultipole

atx emittance computation fails. Likely ohmi envelope internal tracking must also become aware of the new integrators

This is much more difficult: ohmi_envelope does not accept anything else that BndSymplectic4RadPass (no wigglers, no ids…). I even don't know how we can solve this.

missing ExactBendPass and ExactBendRadPass.

ExactBendPass works and give results identical to ExactHamiltonianPass. But there are 2 big problems:

  • enter and exit face angles: I could not yet extract from Forest's book haw to compute them
  • Focusing: though it seems to wrok, I cannot understand the results. For me it ice wrong, unless tests contradict me

@swhite2401
Copy link
Contributor

ohmi_envelope does not accept anything else that BndSymplectic4RadPass (no wigglers, no ids…). I even don't know how we can solve this.

Hello @lfarv, this is a big problem...could we use the development on the 6D optics to generalize ohmi envelop to any element sequence? I find it very disturbing that 2 tracking engines are used in AT....

For the rest I agree with your proposal

@lfarv
Copy link
Contributor

lfarv commented Mar 8, 2023

Hello @lfarv, this is a big problem

Yes and no…

The problem is that ohmi_envelope needs an additional computation when going through an element: on top of computing the output coordinates, it needs some integration of Bperp. This is done in a kind of augmented "copy" of BndSymplectic4RadPass. As you said, it's a kind of double tracking. Integrating Bperp in standard integrators would slow down the tracking, so we would need either to duplicate all integrators, or at least to add a 2nd entry point in each. And find the right computation of Bperp in all cases: not obvious for wigglers and ids, but should be possible for ExactBendPass, since only the inner drifts are concerned, the kicks are identical to the standard ones. This is a major modification of AT's structure, and the 6D developments don't help for that…

However… The emittance computation is not affected by the small difference between "linear" and "exact" passes. We are talking of a single pass around the ring, along the closed orbit (no transverse amplitude). With no errors, it's strictly identical. So the added value of this large development is close to zero… It's more a matter of principle.

@lfarv
Copy link
Contributor

lfarv commented Mar 8, 2023

@simoneliuzzo and @swhite2401 : while thinking of this, I found the reason for the "focusing" problem: ExactBendPass describes a rectangular magnet. That is mechanically rectangular: the magnetic axis (and the AT reference frame) is a straight line. While describing its curved trajectory, the particles experience large deviations from the magnetic axis, and see a much larger quadrupole focusing than in a mechanically curved magnet. You cannot compare with the usual AT bend magnet.

So finally, it seems that the present integrator is right, but it describes a very special kind of magnet. Introducing face angles still looks still possible, but describing a curved magnet means writing a completely different iterator. Without focusing, the shape does not matter, the integrator may be used.

@swhite2401
Copy link
Contributor

So the added value of this large development is close to zero… It's more a matter of principle.

Well don't necessarily agree with that, this is a recurring question (if not a problem) that we get as a feedback on AT. Especially the wiggler and ids (again something that we have talked a lot about) not having any effect on emittance. I would personally find it very uncomfortable to have to switch passmethods in a lattice whether I want to get the correct emittance or the correct phase space.
Finally duplicated functionalities is bad for efficient maintenance centralizing everything in the passmethods would be a significant improvement in this respect.

That being said, it does seem like a big development...

@swhite2401
Copy link
Contributor

Ah and I have a side comment, that should be a much easier fix, why is the C part of ohmi_envelop buried into atmat?? Wouldn't it make more sense to put where all the C code is located?

@simoneliuzzo
Copy link
Contributor Author

for me there is no need to rewrite integrators. We could simply modify the lattice inside ohmi_envelope such as any intetegrator not known by ohmi_enevelope is converted to what ohmi_envelop accepts: ExactBendPass -> BndMPole..., ExactMultipolePass --> StrMPole... , ExactDriftPass --> DriftPass etc... with adequate modification to the help

@simoneliuzzo
Copy link
Contributor Author

exactMultipoleRadPass looks easy to implement. Once this is done, I would suggest merging ExactDrift and ExactMultipole

perfect for me! then we can think of ExactBend in an other PR.

@lfarv
Copy link
Contributor

lfarv commented Mar 10, 2023

@swhite2401, @simoneliuzzo : here is an idea of how we could progress in computing the equilibrium emittance in a more general way. I'm not yet sure it works, but I would like your comments on the idea:

  1. Add a new entry point in the concerned integrators: this new entry point has the same input arguments as trackFunction: particle coordinates and element description. It has an additional output argument, returning the diffusion matrix. Internally, this will call a single integrator function with a flag triggering the computation of diffusion if necessary. This can be tested in BndMPoleSymplectic4RadPass, and then generalised to other integrators. At 1st glance, one could reproduce the diffusion computation for all integrators using a drift-kick sequence: BndMPoleSymplectic4RadPass, StrMPoleSymplectic4RadPass, GWigSymplecticPass. And probably their "Exact" counterparts, to be checked. This development works for both Matlab and python.

  2. In ohmi_envelope, replace findmpoleraddifmatrix by a kind of copy of at.c (python) or atpass.c (Matlab). It deals with a single element (not the whole line as at.c), and has to load the required shared libraries, build the table of entry points, and call the one selected by the element. And returns the diffusion matrix !

Merging what's in findmpoleraddifmatrix and in BndMPoleSymplectic4RadPass is not easy, but looks possible.

@swhite2401
Copy link
Contributor

@lfarv, I think it would be beneficial to centralize as much as possible the C tracking functions

@lfarv
Copy link
Contributor

lfarv commented Mar 14, 2023

@swhite2401 : That's exactly what I propose: a single C file for both tracking and diffusion matrix. The entry points must be separated because the computation of diffusion takes much longer than a simple tracking, and we don't want to significantly slow down the tracking. Inside, a single function will track and optionally compute the diffusion. Both are interleaved, so the merge is tricky, but that can be done.

@swhite2401
Copy link
Contributor

Yes, I was giving you my positive opinion on your proposal... seems tricky as you say but if you think it is possible and would like to take a look, I think it is a nice development

@lfarv
Copy link
Contributor

lfarv commented Mar 14, 2023

I'll make a trial with BndMPoleSymplectic4RadPass. There may be unexpected problems, we'll see !

@lfarv
Copy link
Contributor

lfarv commented Mar 14, 2023

exactMultipoleRadPass looks easy to implement. Once this is done, I would suggest merging ExactDrift and ExactMultipole

perfect for me! then we can think of ExactBend in an other PR.

Done in #581

@simoneliuzzo
Copy link
Contributor Author

I add here an other issue found using pyAT and the Exact*Pass

ring = at.load_lattice(lattice_file, mat_key=ring_var_name)

File "/machfs/liuzzo/FCC/FCC_py38_venv/lib/python3.8/site-packages/at/load/allfiles.py", line 58, in load_lattice
return load_func(filepath, **kwargs)
File "/machfs/liuzzo/FCC/FCC_py38_venv/lib/python3.8/site-packages/at/load/matfile.py", line 169, in load_mat
return Lattice(ringparam_filter, matfile_generator, abspath(filename),
File "/machfs/liuzzo/FCC/FCC_py38_venv/lib/python3.8/site-packages/at/lattice/lattice_object.py", line 185, in init
super(Lattice, self).init(elems)
File "/machfs/liuzzo/FCC/FCC_py38_venv/lib/python3.8/site-packages/at/lattice/lattice_object.py", line 1306, in params_filter
for idx, elem in enumerate(elem_filter(params, *args)):
File "/machfs/liuzzo/FCC/FCC_py38_venv/lib/python3.8/site-packages/at/load/matfile.py", line 115, in ringparam_filter
for elem in elem_iterator(params, *args):
File "/machfs/liuzzo/FCC/FCC_py38_venv/lib/python3.8/site-packages/at/load/matfile.py", line 81, in matfile_generator
yield element_from_dict(kwargs, index=index, check=check, quiet=quiet)
File "/machfs/liuzzo/FCC/FCC_py38_venv/lib/python3.8/site-packages/at/load/utils.py", line 258, in element_from_dict
sanitise_class(index, cls, elem_dict)
File "/machfs/liuzzo/FCC/FCC_py38_venv/lib/python3.8/site-packages/at/load/utils.py", line 254, in sanitise_class
raise err("is not compatible with Class {0}.", class_name)
AttributeError: Error in element 3: PassMethod ExactDriftPass is not compatible with Class Drift.
{'FamName': 'DRIFT_0', 'Length': 2.20022502109568, 'PassMethod': 'ExactDriftPass'}

@lfarv
Copy link
Contributor

lfarv commented Mar 19, 2023

@simoneliuzzo : As usual, in that case, you can bypass the "coherence tests" by using check=False when calling load_lattice. Anyway, this is now corrected in this branch and in #581.

@lfarv
Copy link
Contributor

lfarv commented Mar 20, 2023

There is significant progress on this branch:

  • ExactRectangularBendPass is fully operational,
  • Pole face angles are now taken into account,
  • "RadPass" methods are working: ExactMultipoleRadPass and ExactRectangularBendRadPass.

In addition, all is ready for an ExactSectorBendPass, which will be more similar to the classical methods.

@simoneliuzzo
Copy link
Contributor Author

Dear @lfarv and @swhite2401,

I restored a benchmarking function MADX-PTC vs AT that I wrote in 2017.
It features among others fringe fields and kinematic term.

I am running tests on FCC-ee lattice, I will let you know the results.
I am replacing pass methods as follows:
DriftPass --> ExactDriftPass
StrMPoleSymplectic4Pass --> ExactMultipolePass
BndMPoleSymplectic4Pass --> ExactRectangularBendPass
and their radiative equivalents

let me know if this is correct

@lfarv
Copy link
Contributor

lfarv commented Apr 5, 2023

Hello @simoneliuzzo and @swhite2401

Basically, you test makes sense, however I have some remarks about ExactRectangularBendPass.
I am at the moment busy on ExactRectangularBendPass and ExactSectorBendPass, here is the present situation;

  • ExactSectorBendPass should not be trusted: still problems with symplecticity,
  • ExactRectangularBendPass can be used easily if there is no focusing. If there is focusing, it’s still not fully tested, but there is another difficulty: since the trajectory inside the magnet is no more an arc of a circle, it cannot be predicted at magnet definition. So there is an additional tuning procedure to be done: initially, a reference particle entering at [0;0;0;0;0;0] will get out on a non-zero trajectory, because of the quadrupole effect. Therefore it needs a tuning of PolynomB[0] to recover a zero output (this corresponds to a tuning of the transverse magnet position to get the right deviation). In addition, it also needs a correction of the path length to compensate for the difference of the real reference trajectory and the arc of a circle. An additional attribute to store this correction is not yet available but will be similar to the RefDZ of the BndStrMPoleSymplectic4Pass integrator, see this. I'm planning an elements's method to do this easily, but it's not yet available. ExactSectorBendPass avoids this problem, but as I said, it's not yet ready.

I'll make sure to-morrow that #581 is up to date, while this branch (#574) is my working branch (WIP), so don't trust it !

@lfarv
Copy link
Contributor

lfarv commented Apr 5, 2023

Continuation: I have now implemented all Forest's formulae (the ones from PTC) for rectangular bend, sector bend, edge focusing, fringe field…, but there are still problems: first order is correct (tunes, optical functions…), but I still have problems with horizontal chromaticity and with symplecticity (unexpected anti-damping). Either there are still bugs in coding, or there are errors in my sources (the book and some Forest notes)…

You can have a look at the code in this branch if you are interested ! Everything is in exact* files

@simoneliuzzo
Copy link
Contributor Author

simoneliuzzo commented Apr 6, 2023

Dear @lfarv and @swhite2401

I have some initial comparisons.

here find the phase space plots for the initial case, no exact pass
NoExactPass
NoExactPassY

Then I replace Drifts with ExactDriftPass
ExactDriftPassX
ExactDriftPassY

And then Drifts and Multipoles with the equivalent Exact*Pass
ExactDriftMultPassX
ExactDriftMultPassY

This looks ok in the H plane. less in the Vertical plane. If I sum the distances of all corresponding points in phase space this is the residual

Screenshot 2023-04-06 at 15 31 07

I use only 4D initial coordinates.
Including 6D initial coordinates the comparison is much worst, but it could be due to wrong "coordinate conversion" from madx to AT.
Including radiation also gives problems at the moment.

For those with access to ESRF filesystem, the scripts to produce the figures above are in: /machfs/liuzzo/accelerator_toolbox_tests/matlab/exactpass

@simoneliuzzo
Copy link
Contributor Author

at.load_mat() of a lattice with Exact*Pass passmethods fails in pyAT .
use of at.load_mat(..., check=False) becomes compulsory for the exactpass branch.

@swhite2401
Copy link
Contributor

Should we close this one?

@lfarv
Copy link
Contributor

lfarv commented Nov 29, 2023

@swhite: I think all the issues have been addressed in #581. There is just in a previous comment a discussion about the computation of diffusion matrices which could be moved to a discussion item.

@simoneliuzzo
Copy link
Contributor Author

no objection

@swhite2401
Copy link
Contributor

I opened a discussion on diffusion matrices and close this PR

@swhite2401 swhite2401 closed this Dec 1, 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

3 participants