New Lattice properties: energy, harmonic number and particle

[lfarv]

Following discussions in #319, it appeared that the harmonic number of a ring should be better defined as a Lattice property rather than a RFCavity property. More generally, this PR will add these 3 items to the Lattice properties:

• energy (was already present, but is better handled),
• harmonic number,
• particle, for future use.

The harmonic_number is used to set the RF frequency.
At the moment, no modification is made on the way the Element properties are handled: the HarmNumber field of cavities is ignored, but the Energy field is still necessary and used by the tracking in cavities and radiating elements. The new lattice properties are ready and should be used instead in the future, but this will require modifications of tracking.

A Particle is defined by a name, a rest energy in eV and a charge in elementary charge unit. At the moment, only the 'relativistic' particle is allowed: it has a charge -1 and a rest energy of zero, so its relativistic beta is always 1 which is at the moment the only allowed value for tracking. So any attempt to select another particle raises an error. Tracking of non-relativistic particles is foreseen in the future.

All this required simultaneous modification in many modules, for both python and Matlab. In particular, loading, saving and importing lattices had to be upgraded so that compatibility with existing files is ensured, while newly created files will contain the new information.

So here are the main changes:

python

The standard Lattice properties are now: name, energy, periodicity, harmonic_number, particle.

When building a lattice, the search for these properties is done in the following way, by decreasing priority order:

energy: input keyword argument, then Energy field of the "main" cavity (see below), then Energy field of any other element. If energies of different elements are inconsistent, a warning is emitted. If no energy definition exists, an error is raised.

The "main" cavity is the cavity (or cavities) with the lowest frequency.

Harmonic number: input keyword argument, then HarmNumber field of the "main" cavity, then frequency of the "main" cavity divided by the revolution frequency. If no definition is found (no cavity in the lattice for instance), harmonic_number is undefined.

Particle: input keyword argument, and then defaults to 'relativistic'.

When loading a file, the missing data (in an old file) will be deduced by applying the same search rules.

The 5 properties may be freely modified on an existing lattice.

To be consistent, the harmonic number access is removed from the set_cavity method.

Matlab

The modifications are more important since there is no Lattice class in Matlab. As previously, the lattice properties are stored in a RingParam element, and are handled by the upgraded atGetRingProperties and atSetRingProperties functions. The standard list of properties is now: FamName, Energy, Periodicity, HarmNumber, Particle.

atGetRingProperties handles properly missing or incomplete RingParam elements by applying the search rules described above for python. It is now much more efficient by storing the location of the RingParam element for faster access. If there is no RingParam element, a warning tells that the function is much slower because it needs to scan the whole lattice.

atSetRingProperties creates the RingParam element if it is missing, or upgrades it if necessary, for instance after reading an old file. It also provides an easy way to create or upgrade an incomplete RingParam element with the simple command ring=atSetRingProperties(ring)

[swhite2401]

@lfarv, since we are talking about non-relativistic particles etc... do you think it would be worthwhile introducing a beam object that would handle and carry the particles, each particle could then have its own properties such as mass, charge, coordinates, etc...
I believe this would provide much more flexibility than the present implementation and allow to track at the same time several ion species. This could be interesting for target interaction studies for instance.

[lfarv]

@swhite2401: this looks much more ambitious than what is proposed here ! Why not, but I don't think it interferes with this PR: most of it deals with handling of harmonic number and code cleaning of the Lattice object. The introduction of the Particle object is straightforward, and the only missing part to handle non-relativistic particles is a modification of RFCavityPass, which is a minor point (though I have trouble with it at the moment).

Such a beam object would come on top of that, with several options for tracking: either track separately the different ion species, or heavily modify the tracking engine to add mass and charge as additional coordinates, or as additional variables… This needs to be carefully imagined, including the use case: AT is restricted in magnets to small angles and deviations, it's not meant to design spectrometers…

[swhite2401]

Absolutely, it was just a suggestion! this PR is perfectly fine for me.

[swhite2401]

@lfarv , I started using this branch for some other development and realized I went through this one a bit too fast:
-I saw that you set the rf frequency and voltage as lattice properties and I disagree with that: these are cavity properties. Having the getter and setter methods as Lattice attributes is sufficient in my opinion.
-Then I got really confused by ring.revolution_frequency and ring.get_revolution_frequency. Similarly I would keep only get_revolution_frequency as a lattice attribute (it give more possibilities), or at least change the names. As is it now it really seems the 2 should give the same value...

[swhite2401]

Also I think set_rf_frequency belongs to cavity_access with same interface as other cavity attributes. We may add a more advanced method as you proposed in revolution.py that we may extend with tracking in the presence of radiation etc and call tune_rf_frequency for example.

[swhite2401]

I am working on an improved cavity_access module that give more flexiblity than the present one, I can integrate these suggestions in this new PR

[lfarv]

-I saw that you set the rf frequency and voltage as lattice properties and I disagree with that: these are cavity properties. Having the getter and setter methods as Lattice attributes is sufficient in my opinion.

I agree that the frequency is a cavity property, however for modifying the frequency on all cavities in one shot, I do not see any other possibility than applying it to the ring ! Now for the naming, I just follow the common practice of having getter and setter methods prefixed with 'get_' and 'set_' and the corresponding property without any prefix.

-Then I got really confused by ring.revolution_frequency and ring.get_revolution_frequency. Similarly I would keep only get_revolution_frequency as a lattice attribute (it give more possibilities), or at least change the names. As is it now it really seems the 2 should give the same value...

Same answer as above: to be consistent, same convention (except there is no setter function). And yes, they should give the same result, unless I left a bug…

In all cases, it's true that the property does not bring any new functionality compared to the methods, it's just more convenient is most cases (when using the default arguments). And you are free to ignore them if you prefer the methods.

[swhite2401]

-I saw that you set the rf frequency and voltage as lattice properties and I disagree with that: these are cavity properties. Having the getter and setter methods as Lattice attributes is sufficient in my opinion.

I agree that the frequency is a cavity property, however for modifying the frequency on all cavities in one shot, I do not see any other possibility than applying it to the ring ! Now for the naming, I just follow the common practice of having getter and setter methods prefixed with 'get_' and 'set_' and the corresponding property without any prefix. Having different names would be much more confusing.

-Then I got really confused by ring.revolution_frequency and ring.get_revolution_frequency. Similarly I would keep only get_revolution_frequency as a lattice attribute (it give more possibilities), or at least change the names. As is it now it really seems the 2 should give the same value...

Same answer as above: to be consistent, same convention (except there is no setter function). And yes, they should give the same result, unless I left a bug…

In all cases, it's true that the property does not bring any new functionality compared to the methods, it's just more convenient is most cases (when using the default arguments). And you are free to ignore them if you prefer the methods.

Again for multiple RF systems the logic is broken as there is no single frequency and this is misleading for standard user that did look at the source, I would keep only the getter and setter which are unambiguous.

As for ring.revolution_frequency as far as I could understand this is fixed and computed from the reference path length, right? On the other hand get_revolution_frequency takes input arguments so results may differ... am I missing something?

[lfarv]

Also I think set_rf_frequency belongs to cavity_access with same interface as other cavity attributes. We may add a more advanced method as you proposed in revolution.py that we may extend with tracking in the presence of radiation etc and call tune_rf_frequency for example.

In principle, that's true. But setting the frequency needs to know the revolution frequency. And I could not keep the revolution frequency in the lattice package without removing the possibility of selecting dp or dct. I don't like the idea of duplicating functions for the same purpose. As a consequence, set_rf_frequency has to be moved to the lattice package. I agree it looks strange, but it has no real drawback. Another possibility is to move the whole cavity_access module to the physics package. Why not… It's probably still time for that.

Ok for improvements, but keep the interface clean: having both set_rf_frequency and tune_rf_frequency is confusing if a single one fits.

[swhite2401]

Ok then I propose we merge this one as is, because it is fine for the moment, then we can continue the discussion when I have something to propose.

[lfarv]

Again for multiple RF systems the logic is broken as there is no single frequency and this is misleading for standard user that did look at the source, I would keep only the getter and setter which are unambiguous.

Do you really think that a statement like:
ring.rf_frequency = 500e6 or ring.rf_frequency = Frf.NOMINAL could be misleading to anybody ?

For multiple RF systems, the cavpts argument identifies which system you are controlling. And again, the property is more convenient if either there is a single system, or if the main system is identified by the optional cavpts attribute of the Lattice (the web documentation is clear on that). The property is equivalent to the setter method with default arguments, and as I said, it should cover most of the cases.

As for ring.revolution_frequency as far as I could understand this is fixed and computed from the reference path length, right? On the other hand get_revolution_frequency takes input arguments so results may differ... am I missing something?

No, you don't miss anything: the property corresponds to the default arguments of the method, as usual.
In that particular case, ring.revolution_frequency is the revolution frequency of on-momentum particles, which seems rather obvious to me. I do not imagine how someone could misinterpret it.

In both cases, the meaning of the property is so obvious that I do not see how it could be interpreted other than what it says.

[lfarv]

@swhite2401: do you think it's worth moving the cavity_access module to the physics package and reintegrating set_rf_frequency in it? It's not entirely logical either, it makes more sense in lattice, but I do not see any perfect solution.

[swhite2401]

Definitions are clear, I think what confused me is that the property is defined in one place and the getter somewhere else,
why don't you define ring.revolution_frequency = property(get_revolution_frequency, None, None, 'doc') in revolution.py instead of building it in Lattice_object.py? It also makes it easier for the reader to have everything in one place.
The same applies to ring.frequency but it s more tricky as there is this import ordering issue...I like to have cavity_access in lattice it clearly makes more sense there I agree.

I'll think about it...as I said lets merge this one as is. More to come on RF tuning...

[lfarv]

why don't you define ring.revolution_frequency = property(get_revolution_frequency, None, None, 'doc') in revolution.py instead of building it in Lattice_object.py?

Good idea, I just looked at it. The problem is that the Lattice.__init__ uses it to set the harmonic number… One could duplicate the computation of the revolution frequency to avoid that, it's short enough. But I don't like duplicate things…

[swhite2401]

why don't you define ring.revolution_frequency = property(get_revolution_frequency, None, None, 'doc') in revolution.py instead of building it in Lattice_object.py?

Good idea, I just looked at it. The problem is that the Lattice.__init__ uses it to set the harmonic number… One could duplicate the computation of the revolution frequency to avoid that, it's short enough. But I don't like duplicate things…

Ah too bad...so in this case do we really need the harmonic number to be defined at initialization of the Lattice object? We may as well define getter and setter in revolution.py and follow the same convention as for cavity attributes. Is there any benefit to the present implementation?

[swhite2401]

@lfarv can you merge this one? Anything blocking it? I would need it for other devs...

[lfarv]

@swhite2401 :I'm planning to implement the move of Lattice.revolution_frequency that you suggested. Later to-day, I think…