Bug: atwiggler

[swhite2401]

We are still suffering from the bug reported in #190. atwiggler uses GWigSymplecticPass as default, which will then point to GWigSymplecticRadPass when 6d is enabled. The latter has a bug and shall not be used. Not only does it have a bug but now fails to run with atpass complaining about a missing field energy even though it is defined.

We have 2 severe issues here:

1. We are distributing a passmethod with a know bug that users still use and suffer from (I was recently contacted by an NSLSII colleague regarding this issue)
2. The default at wiggler creation function points by default to this passmethod with know bugs

This situation is really not appropriate has to be solved rapidly by either removing the function with bugs (or at least make the necessary changes to not have it as default wiggler function) or by fixing it which obviously take much more efforts.

I have no problem removing it, as far as I am concerned we should not distribute function giving the wrong results.
Could you please advise?

Thanks!

[ZeusMarti]

Hi @swhite2401,

As I mentioned earlier we are working on this point for our implementation at ALBA. We are basically adapting @mashl version which for him is working. As I said before, my understanding is that the current GWigSymplecticRadPass is not accompanied with the corresponding additional files and modifications in the ohmienvelop.m function. I guess @ahmad should update his pull requests but he seems quite busy and actually I'm not sure his is reading this (despite my efforts).

In summary, in case it helps we will let you know if it finally also works for us.

Best,

Zeus

[swhite2401]

Thanks @ZeusMarti it is in fact excellent news that you are tackling this issue. The question is now whether we should disable this feature until a working solution is found or at least issue a clear warning message to prevent any confusion on the users side.

[ZeusMarti]

Hi @swhite2401,

As I mentioned earlier we are working on this point for our implementation at ALBA. We are basically adapting @mashl version which for him is working. As I said before, My understanding is that the current

[ZeusMarti]

I believe a warning should suffice, just in case my opinion counts.

[lfarv]

Hello @swhite2401 and @ZeusMarti,

There are 2 problems with GWigSymplecticRadPass:

1. For tracking, the present implementation is wrong: it's missing the change in momentum. As it is, there is no momentum change due to radiation, which is obviously wrong. This is easy to correct, since GWigSymplecticRadPass is based on a drift-kick sequence in cartesian geometry, the same as what is used if StrMPoleSymplectic4RadPass. So all ingredients are available,
2. For equilibrium emittance, the problem is more general: unlike tracking, which is modular, the computation of the diffusion matrix is monolithic, done in findmpoleraddiffmatrix.c. This function is just an extension of BndMPoleSymplectic4RadPass, adding the propagation and increment of the diffusion matrix. A general solution consists in making this modular, and to avoid building a new family of specific functions, a proposed solution is to integrate the processing of diffusion matrices as an option in the existing pass methods. After the discussion in General computation of diffusion matrices #715, a prototype of this big change is already available in the Modular computation of diffusion matrices #742 pull request. It's fully functional, but such a big modification of the structure of AT deserves some discussion, so I don't expect the branch to be merged in a near future. As the branch is now, findmpoleraddiffmatrix.c is replaced by something similar to at.c, which distributes the task depending on the element passmethod. I encourage you to have a look at this branch.

For the demonstration, up to now only BndMPoleSymplectic4RadPass, StrMPoleSymplectic4RadPass and ExactMultipoleRadPass have been adapted to propagate diffusion matrices (which is already better than the present computation). As I said above, GWigSymplecticRadPass is based on existing integration steps, so it should be easy to adapt it. I'll be leaving soon up to the end of April, but I think I could have an upgraded version of GWigSymplecticRadPass during May, if you are interested. But still a prototype for evaluation, while important design decisions will still have to be made.

After working a bit on this branch, I intend to post in a few days a summary of the remaining difficulties (mainly the "exact" passmethods), and of some choices to be made.

In the mean time, I can remove GWigSymplecticRadPass and remove wigglers from the default elements switched on in "enable_6d". Or make the simple correction for momentum loss in the present version, forgetting the diffusion, but I'm not so motivated to work on something which should be replaced soon. Let me know…

[swhite2401]

Hello @lfarv, I agree that there is no need to put to much effort on something that will be replaced. For me adding a warning message is fine as well.

Just for my understanding, from previous discussion I thought that this element was also modifying dispersion which in principle is not supposed to happen in a wiggler, could you confirm this?

[lfarv]

Just for my understanding, from previous discussion I thought that this element was also modifying dispersion which in principle is not supposed to happen in a wiggler, could you confirm this?

Sorry, but I can't remember that… Does this mean that GWigSymplectPass (no Rad) would also be wrong? I was thinking of adapting it for radiation and diffusion, but I did not think of reviewing its basis…

For me adding a warning message is fine as well.

to @oscarxblanco and @swhite2401 : ok, but when should this message appear ? In Lattice.enable_6d ? And if so, should we keep wigglers in the set of elements turned on by default ?

[swhite2401]

I was thinking more in the C when the element is loaded in atpass but this is not necessarily the best option

[joanarenillas]

Hello @swhite2401 and @lfarv,
I am Joan Arenillas and I am currently working with @ZeusMarti and @mashl on the implementation of wiggler diffusion matrices in ohmienvelope. We are also implementing momentum change due to radiation in GWigSymplecticRadPass, and are planning on presenting the new codes soon.

As @swhite2401 said in the first comment in this issue, there is a bug with atx when using a lattice with a wiggler. In particular, the function atdisable_6d removes the Energy attribute to every element in the lattice (we have tested this with the lattice of ALBA).

Could you please advise on how to tackle this issue?

Thanks!

[swhite2401]

Hello @joanarenillas and welcome!

I think there are several ways to go around this:

1. You may get the ring energy accessible in the C through the param struct in the input arguments of the trackFunction. For information, the parameters available in this structure are in atintegrators/attypes.h
2. In case you really want the energy to be a users input, i.e. an attribute of the element, it is a bit more complex. The wiggler element inherits from the _Radiative class (in elements.py). You can see there that set_long_motion will delete the Energy attribute when disabling the 6D motion. You may override this function in the Wiggler class with the proper behavior to keep the attribute Energy even with 6D disabled

Not sure if that makes sense... please let me know if you need more details or help. I can also look at your branch an comment directly in the code if this makes it simpler for you.

There might be other option I did not think of, @lfarv any ideas?

[lfarv]

Hi all,Just a quick answer to let you know that the ring energy is already available in C in the “param struct”. It’s where you have to take it. LaurentLe 17 avr. 2024 à 12:15, Simon White ***@***.***> a écrit : Hello @joanarenillas and welcome! I think there are several ways to go around this: You may get the ring energy accessible in the C through the param struct in the input arguments of the trackFunction. For information, the parameters available in this structure are in atintegrators/attypes.h In case you really want the energy to be a users input, i.e. an attribute of the element, it is a bit more complex. The wiggler element inherits from the _Radiative class (in elements.py). You can see there that set_long_motion will delete the Energy attribute when disabling the 6D motion. You may override this function in the Wiggler class with the proper behavior to keep the attribute Energy even with 6D disabled Not sure if that makes sense... please let me know if you need more details or help. I can also look at your branch an comment directly in the code if this makes it simpler for you. There might be other option I did not think of, @lfarv any ideas? —Reply to this email directly, view it on GitHub, or unsubscribe.You are receiving this because you were mentioned.Message ID: ***@***.***>

[joanarenillas]

Dear @swhite2401 and @lfarv,
Thank you for your insights. The trackFunction in GWigSymplecticPass and GWigSymplecticRadPass were wrongly defining the energy from "ElemData", instead of taking it from "param struct" as you suggested. I will modify this in my branch.

I have noticed that all pass methods with radiation set the energy from the "param struct" in their Track Function (Matlab and Python) but instead take the energy from "ElemData" in their Matlab Mex Function. Is this difference some compiler requirement or should we maybe change this so both the Track and Mex Functions take the energy from the "param struct"?

Regards

[swhite2401]

Dear @joanarenillas , this is a good point. I think there is a historical reason to this: pass methods were implemented before the param struct was introduced.
You are right we may consider to clean this up. @lfarv , any objection? I think this is a good proposal.