Handle diffusion element in radiation_off()

[swhite2401]

As suggested in #451 a separated PR is created to introduce a QuantDiffElement and set its passmethod to IdentityPass or QuantDiffPass using Lattice.radiation_off/on()

[lfarv]

The suffix "Element" is usually not part of the class name of Elements (ex.: "Dipole", "RFCavity'",...). So why not using "QuantumDiffusion" as class name ?

[lfarv]

It is also probably necessary to turn off WakeElements to remove all momentum changes. The list of such elements may increase. Can we afford adding cases and keywords in radiation_off, or is there a more clever way to do ? Anyway, we can merge as is in a 1st step, and think about it later.

[lfarv]

Whatever we do, we have to modify atradoff (Matlab) to keep in synch both versions… I can take that if you like !

[swhite2401]

Yes please do! You may allow rename the class, I agree it is not consistent with other elements

[swhite2401]

For the WakeElement, I don't know...you are probably right but then this gets tricky if we add to many cases...

By the way I forgot to modify the check_radiation() decorator.

[lfarv]

The branch is now a major update: since it looked difficult for the Lattice object to handle in radiation_on for all possible new elements, radiation_on is now delegated to the Element object. The Element object has one new method:

• set_long_motion(onoff: bool, copy: bool = False)

And 2 new properties:

• long_motion: True if the element may modify the momentum of the particle
• collective: True if the element deals with collective effects

The value of these properties depend of course on the longitudinal motion status.
The term "radiation" used up-to now is replaced here by "longitudinal motion" which is more general.

In the Element base class, the set_long_motion method does nothing, and both properties return False.

We also introduce 3 mixin classes: LongMotion, Radiative and Collective. Simply adding one of these classes to base classes of an element class will automatically make the it handle the on/off status without any other modification.

The Radiative class must be used for radiative elements (at the moment Multipole and Wiggler). It switches the suffix of the PassMethod between Pass and RadPass,

The LongMotion class switches the PassMethod between IdentityPass and the element default PassMethod (used at the moment for QuantumDiffusion and RFCavity, with small modifications)

The Collective class is similar, but in addition turns the collective property True when the element is in the "ON" state. Used at the moment for WakeElement.

Summary:

To handle radiation_on/off in a new Element subclass, simply put LongMotion, Radiative or Collective at the head of its base classes.

[lfarv]

The Lattice object has a new property collective, true if the collective property of any of its elements is True.
The radiation_on / radiation_off methods now simply call the set_long_motion method of the selected elements. They have new keywords:

• other_pass: modifies elements modifying the momentum but not concerned by the existing keywords. Used for QuantumDiffusion and potential new elements. Default: 'auto',
• collective_pass: modifies all elements dealing with collective effects. WakeElement at the moment. Default: 'auto',
• all_pass: defines the default behaviour for all elements not explicitly specified. Example:

  newring = ring.radiation_on(all_pass=None, cavity_pass='auto')

  turns cavities ON but leaved everything else unchanged.

[lfarv]

@swhite2401 : I cannot put you in reviewers since you initiated the PR… Can you have a look?

[swhite2401]

@lfarv, this is indeed a major change and very difficult to review... I hope you have done all the proper testing...

I have several comments:

• other_pass should be changed to 'quantdiffusion_pass` or what ever it points to otherwise it is impossible to understand for the user
• set_radiation() decorator for optics uses the default behavior that turns on collective effects and quantum diffusion, I suppose you would like to avoid that
• In fact this version is changing the AT default behavior and mixes even more functionalities into radiation, I would keep the default behavior for collective effects and diffusion to None for radiation_on(), on the other hand I agree that radiation_off() has to turn everything off
• Since you are at it, I think it would be very useful and easy to add other functions such as cavities_on/off(), collective_on/off(), radiation_damping_on/off(), diffusion_on/off(), 6dmotion_on/off(). I think there has been a lot of discussion on this and we all agreed that the name radiation_on/off() was very poorly chosen, adding such functions may remove some of these ambiguities while maintaining the wrongly named function and backward compatibility.

I might have more comments later

[lfarv]

Hi again. I won't be available too much for the few next days, so I'll comment on a few points you mentioned, and we'll have some time to think about it:

other_pass should be changed to 'quantdiffusion_pass`

I agree, the other_pass keyword is bad. The idea is to group there all elements modifying the momentum and not covered by more specific keywords. From now on, there should. be no need to change the function any more for future elements. Apart from quantum diffusion, this could cover for instance longitudinal fast kickers if ever…

If you have a good idea, I take it!

Similarly, collective_pass group any future collective effects element in addition to the present ones. (It's true that transverse impedances don't modify the momentum, but if you have to turn off longitudinal motion, for mcf computation for instance, there is no interest in keeping transverse impedances, so I take them off together)

[lfarv]

I think it would be very useful and easy to add other functions such as cavities_on/off()

Good idea. I'm not fan of numerous keywords, and as I wrote above, I would not like to add more than what is proposed here. So specialised functions are welcome.

Concerning the function name, radiation_on is indeed misleading, and you'll notice that for the equivalent methods and properties of the Element object, I'm now referring to "longitudinal motion". Unfortunately, we cannot change the name any more. But a possible improvement is to create an alias for the function, long_motion_on(), for instance, or similar. The 6dmotion_on() that you suggest would be ideal, but is unfortunately not allowed as function name. We keep the present name for compatibility, and add on the doctoring a something like "Obsolete, use long_motion_on() instead".

I don't consider that this PR introduces "new functionalities" in the function: it simply tries to deal with new elements modifying the particle momentum, and therefore inducing longitudinal motion.

So we can prepare a list of new functions, starting with the most useful ones, additional ones may be added later.

[lfarv]

In fact this version is changing the AT default behaviour

I don't think so. Here is why:

1. A QuantumDiffusion element is active when you create it (same for collective effects). At the moment, if you turn "radiation" OFF and then ON again, it is still active in the end (of course, it is not affected). With the new version, it is exactly the same when using default parameters, but in addition it is correctly temporarily turned OFF. Having it disabled in the end would be surprising and dangerous,
2. The default parameters should correspond the most common use. For me, the simplest definition is "everything ON" and "everything OFF". If you introduce quantum diffusion or other fancy elements in your lattice, I assume that you want to use them. And the decorator has to follow the default behaviour,
3. The present proposal gives the best symmetry between ON and OFF. The only dissymmetry concerns radiation in sextupoles and higher multipoles. But unlike QuantumDiffusion, these are not initially radiating (at element creation), and in normal conditions should never be turned ON. The safe choice of turning them OFF in radiation_off is because if there is a possibility to turn them ON, sooner or later someone will do it !

Finally, you may notice that for acting on a single class, like controlling collective effects, the new all_pass keyword makes thongs much simpler: set all_pass=None, specify the class you want to modify and that's all. You don't need any more to remember the default behaviour of the other classes.

Anyway, this will wait for a few days before we make the final decision, and it's 2 lines to change when we decide… Maybe @MJGaughran and @lnadolski can give there opinion?

[swhite2401]

The idea is to group there all elements modifying the momentum and not covered by more specific keywords

The problem with grouping is that we lose the possibility to control things individually, for instance if you group quantum diffusion with your longitudinal kicker, how would you turn one on and not the other (using this function of course...).
I am quite certain users will want this.

How about we define a dictionary, ring.ring_configuration that contains all these keywords, and potentially more that radiation_on/on would take as argument, the user could then easily set things up the way he likes and keep this configuration.

[swhite2401]

I don't consider that this PR introduces "new functionalities" in the function: it simply tries to deal with new elements modifying the particle momentum, and therefore inducing longitudinal motion.

What I am saying is that now you control suddenly a lot more elements with this function, I gave an example in my earlier comment: the set_radiation() decorator uses the default radiation_on(). Before this would turn on radiation damping and rf cavities. Now it will also turn on diffusion and collective effects. The first will give you random results and the second completely wrong ones.
I am quite sure many scripts and functions (including mine) assume this default behavior (radiation damping + cavities), and this PR is changing it. Even worst, it will keep changing over time due to this other_pass keyword that can contain basically anything and can grow infinitely...

[swhite2401]

We keep the present name for compatibility, and add on the doctoring a something like "Obsolete, use long_motion_on() instead".

I would even issue a deprecation warning

[lfarv]

Concerning individual control, alternatively to the (possibly infinite) set of specialised functions, I have another proposal: a new method enable_6d(elemclass: type, pass_method: str = 'auto') (and disable_6d, of course). elemclass, the class of the concerned elements, can be:

• an Element subclass: Dipole,
• a grouping class: Collective, to take as example one of the recently introduced classes.
  Using the concept of mixin classes, it's easy to add new grouping classes for any new purpose.
• a set of such classes: {Radiative, RFCavity}, to mimic the current default. As another example, disable_6d(Element) will kill everything.

As soon as there are several Element subclasses involved, only 'auto' makes sense, an explicit pass_method usually does not apply. Ideally one could also choose None as the default, meaning "default choice of the pass_method", since there is no need to specify "do nothing". But that would be confusing as this convention is opposite in radiation_on .

It could also be a single method set_6d(onoff: bool, elemclass: type, pass_method: str = 'auto') for both ways.

This is simple, self-explanatory and accepts new element classes without modification.

[lfarv]

I would even issue a deprecation warning

Ha ! My previous attempts to set warnings to push users to write better code always triggered a bunch of negative comments: "My code was working well and now I get warnings, please remove that"… So I'm very cautious. Here it's even not an incitation to improve the code: there is no benefit in changing the function to the new one, they are identical.

It's only a good practice in new code to use the new meaningful name instead of the confusing old one.

Anyway I'll implement the alias.

[lfarv]

What I am saying is that now you control suddenly a lot more elements with this function, I gave an example in my earlier comment: the set_radiation() decorator uses the default radiation_on(). Before this would turn on radiation damping and rf cavities. Now it will also turn on diffusion and collective effects. The first will give you random results and the second completely wrong ones.
I am quite sure many scripts and functions (including mine) assume this default behavior (radiation damping + cavities), and this PR is changing it. Even worst, it will keep changing over time due to this other_pass keyword that can contain basically anything and can grow infinitely...

I understand, but if you apply your script right now on a lattice with quantum diffusion and collective effects, they will also be turned on: they are on for ever. And you'll get random or wrong results. The new default does not change anything. On the other hand, with your proposal, a single OFF + ON sequence will kill your diffusion or collective effects, without notice. No perfect solution.

Concerning the drawbacks of grouping, I agree with you, but the purpose of these methods is to globally kill and restore the longitudinal motion. Pushing to the extreme, for acting on a single element, you still need to take it individually.

Important point also: most of these problems may be mitigated by using copy=True: then the notion of restoring the previous state does not hold any more. Selecting False as the default was in my opinion a bad decision from the beginning. But it's excluded to change that now.

Let's wait for other opinions, we'll see I a few days.

[swhite2401]

Let's wait for other opinions, we'll see I a few days.

No perfect solution in fact...maybe someone comes up with better ideas

[swhite2401]

Ha ! My previous attempts to set warnings to push users to write better code always triggered a bunch of negative comments

Right, I take it back...but at some point we will have to remove useless function. Anyway deprecation warnings only makes if we increment versions

[MJGaughran]

Why would you only want to act on the 'standard' set, and not all classes 'involving synchrotron radiation'? If enabling radiative effects does nothing on higher-order multipoles, why do they have the 'Rad' string in the Pass Method?

Happy for you to leave the code as-is, but better docs are always appreciated.

[lfarv]

Why would you only want to act on the 'standard' set, and not all classes 'involving synchrotron radiation'?

The "StrMPoleSymplectic4Pass" method is used for all multipole orders, from quadrupole to infinite order. So it has to have a corresponding "Rad" method. As a consequence it has always been possible to turn on radiation on a dodecapole, let's say. I do not want to remove this possibility. However, in normal conditions, there is strictly no interest in doing that: you increase a lot the computing time and get no benefit. So the present radiation_on method does not turn them on. By default, it acts on dipoles and quadrupoles, and has options for sextupoles and octupoles. This is what I call the "standard" behaviour, which I want to keep. The new class organisation tries to formalise that: the private class defines the internal functionality (modifying the pass method), while the public classes allow the user to select the desired elements:
(Radiative) represents the present default behaviour,
(Radiative, Sextupole) adds sextupoles to the set, and so on.

To turn on everything (strongly discouraged), there is at the moment the multipole_pass keyword, which in the new form is equivalent to specifying Multipole as the base class.

I don't think there is any need to advertise the private _Radiative class, but it could be renamed if you think it would be more clear, Any idea ?

[MJGaughran]

I think the code is fine. This just needs to be mentioned somewhere such as the Radiative class docstrings etc.

[lfarv]

@swhite2401:

One question, have you checked the places in the source where radiation_on() is used? I think most of them assume the 'old' default behavior

Apparently there are very few occurrences of radiation_on() (less than radiation_off()):

• in decorators (@set_radiation), the behaviour is the expected one: use the default options,
• in calls to ohmi_envelope: ohmi_envelope gives crazy results if used with quantum diffusion, and it will be the same with the new behaviour (confirmation that it does not change anything). The difference is that we can now tune it to be correct (not done yet).

Now take your time to test, it's finally a critical update!

[lfarv]

@swhite: is this now OK for you ?

[swhite2401]

@swhite: is this now OK for you ?

Hello @lfarv, sorry I was away until now. I don't want to block this, I looked at the code and this is all fine for me. Please go ahead.