atcollab / at

Accelerator Toolbox
Apache License 2.0
48 stars 31 forks source link

Exact pass #574

Closed simoneliuzzo closed 9 months ago

simoneliuzzo commented 1 year ago

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 commented 1 year ago

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

simoneliuzzo commented 1 year ago

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 commented 1 year ago

missing ExactBendPass and ExactBendRadPass

simoneliuzzo commented 1 year ago

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

swhite2401 commented 1 year ago

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 commented 1 year ago

@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:

swhite2401 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

@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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

@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 commented 1 year ago

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

lfarv commented 1 year ago

@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 commented 1 year ago

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 commented 1 year ago

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

lfarv commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

@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 commented 1 year ago

There is significant progress on this branch:

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

simoneliuzzo commented 1 year ago

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 commented 1 year ago

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;

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 commented 1 year ago

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 inexact* files

simoneliuzzo commented 1 year ago

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 commented 1 year ago

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.

lfarv commented 1 year ago

This check when reading .mat files is awful. And as the file cannot be loaded, it's even impossible to change the passmethod, should one want to do it… I propose to remove the check.

lfarv commented 1 year ago

@simoneliuzzo:

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.

Corrected in #600

swhite2401 commented 9 months ago

Should we close this one?

lfarv commented 9 months ago

@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 commented 9 months ago

no objection

swhite2401 commented 9 months ago

I opened a discussion on diffusion matrices and close this PR