Orbit correction and Orbit Response Matrix measurement through pySC.

[kparasch]

No description provided.

[kparasch]

@JeanLucPons First ORM measurement (on design) is working. You can also check the interface in pyaml/external/pySC_interface.py.

I have created an OrbitResponseMatrix object (see pyaml/tuning_tools/orbit_response_matrix.py) which I would like to attach to the simulator/controlsystem in order to be able to do something like sr.live.orm.measure(). Do you have any recommendation how we could do that?

Maybe it's best we keep it detached in this PR and we attach it in a new one after this one is reviewed and merged?

[JeanLucPons]

Thanks @kparasch , I'm impressed !
I will try this out today on our VA and on a lattice with errors.

[JeanLucPons]

I just tried your example.
It worked. Nice !
At very first glance:

orm = OrbitResponseMatrix(ORM_ConfigModel(element_holder=element_holder, bpm_array_name='BPM',
                                          hcorr_array_name='HCorr', vcorr_array_name='VCorr',
                                          corrector_delta=1e-6))

Your model should not have the element holder, neither the arrays.
For the config I expect that you have only the delta and eventually other stuff such has waiting time, additional task to perform in between each orbit measurement, etc,... All things that are not attached to either a live or a design.

For instance you should be able to do:

hmags = sr.design.get_magnets("HCorr")['SF2*']
vmags = sr.design.get_magnets("VCorr")['SF2*']
bpms = sr.design.get_bpms("BPM")
orm.measure(hmags,vmags,bpms)

It would be nice also to be able to save data to npy format.

orm.save(parent_folder / Path('ideal_orm.npy'))

I continue with test.

[simoneliuzzo]

Dear @kparasch and @JeanLucPons,

very nice developments!

Ideally, if pyaml config is complete we could need to run just orm.measure() (such that any lab can use the code our of the box) with default the mandatory arrays of pyAML Hcor, Vcor, BPM, or you can pass a subset as shown above by @JeanLucPons.

ciao
Simone

[JeanLucPons]

OK, i tested the orbit correction, it went perfectly fine.

Concerning configuration:

OK, so we could keep the configuration proposed by @kparasch but without the element holder.

If we want to follow the specification:

# The orm object should be attached as all other elements in both ControlSystem and Simulator.
sr.design.orm.measure() 
sr.live.orm.measure()

but this prevents from having the writing I proposed with free arrays.

Otherwise we can pass the holder as input param.

orm.measure(sr.design) #  Measure ORM on design using arrays in config
orm.measure(sr.live) #  Measure ORM on live using arrays in config
orm.measure(hCorrectors=hmags, vCorrectors=vmags, bpms=bpms) # Measure ORM using dynamic arrays previously attached by the users

[kparasch]

Be aware orbit correction is still in a very preliminary state, another object will follow soon.

The element holder I only have it there until we attach it. Unless we decide we don't want to do that.

I like the idea of the dynamic arrays but I also would like a default configuration which is specified in the config files.
How about we have the config still use array names, but we then just pass the arrays to the OrbitResponseMatrix. Then we can override the default configuration as in:

orm.measure() # measures ORM with default configuration
orm.measure(hCorrectors=hmags, vCorrectors=vmags, bpms=bpms) # Measure ORM using dynamic arrays previously attached by the users

PS: Do you think we could add MagnetArrays? It would simplify even more some things.
I looked a bit how we could do it but we would have to create a new name for the array. Maybe an array of MagnetArray that behaves as MagnetArray could work?

[simoneliuzzo]

Dear @kparasch,

in fact in the specifications it is requested to be able to do something like:

sr.design.orm.measure() # reference ORM
sr.live.orm.measure() # real accelerator ORM

for the hcor, vcor and bpm defaults it is ok for me. We could have additional arrays to be defined in the config file to specify the default correctors for ORM so the code becomes sharable among labs transparently.

[JeanLucPons]

OK so let's follow the specification and forget free arrays to be passed an input parameters unless a real need is identified.

[simoneliuzzo]

We will need to put this informations in the configuration file for example defining ORM_HCOR, ORM_VCOR arrays of correctors (with less than all correctors) and use them to specify a property "correctors" of the ORM device.

[JeanLucPons]

We will need to put this informations in the configuration file for example defining ORM_HCOR, ORM_VCOR arrays of correctors (with less than all correctors) and use them to specify a property "correctors" of the ORM device.

Yes this is already done by the model proposed by Kostas.

[JeanLucPons]

I don't know waht happened this morning tests was ok and now, it fails in horizontal !
The only diff with original code are few time.sleep(). and ebs = sr.live.

[ubuntu20acu.pons] > python correct_orbit.py 
21 Nov% 2025, 14:06:39 | INFO | Tango control system binding for PyAML initialized with name 'live' and TANGO_HOST=ebs-simu-3:10000
R.m.s. orbit before correction H:  113.3 µm, V:  33.9 µm,
R.m.s. orbit after correction H:  226.5 µm, V:  0.6 µm,

[kparasch]

I suspect it is a sign problem, your EBS uses the sign of kickangle and I think we use the sign of PolynomB which are opposite.

[JeanLucPons]

I suspect it is a sign problem, your EBS uses the sign of kickangle and I think we use the sign of PolynomB which are opposite.

I will check this.

[gubaidulinvadim]

I think we need arrays to be passed as arguments, at least at this stage. The specification also tells you that the user should be able to remove corrector or a bpm from the measurement. This is either a "free" array that is passed as an argument or some standard pyAML array (which does not exist yet in the code). I would not say that it will be convenient for the user to change the configuration file all the time. The config file we generate for SOLEIL II is enormous with 10 000+ lines.

[JeanLucPons]

I think we need arrays to be passed as arguments, at least at this stage. The specification also tells you that the user should be able to remove corrector or a bpm from the measurement. This is either a "free" array that is passed as an argument or some standard pyAML array (which does not exist yet in the code). I would not say that it will be convenient for the user to change the configuration file all the time. The config file we generate for SOLEIL II is enormous with 10 000+ lines.

This should be handled at the Element level with disabled(). If I remember well the specification.
So to disable a BPM should be able to do : sr.live.get_bpm(" BPM_C16-07").disable()

[JeanLucPons]

I suspect it is a sign problem, your EBS uses the sign of kickangle and I think we use the sign of PolynomB which are opposite.

I don't see a possible quick fix if this is this sign problem. Would it possible for you to add 2 separate gain for h and v ?

[kparasch]

@JeanLucPons I don't have the separation between h and v yet in pySC. A quicker fix to see if this is indeed the issue could be if you try:

response_matrix.matrix[:,len(response_matrix.input_names)//2:] *= -1

or if you pass an empty vcorr array to calculation of the ideal ORM, which you can then use with a gain=-1

[JeanLucPons]

@JeanLucPons I don't have the separation between h and v yet in pySC. A quicker fix to see if this is indeed the issue could be if you try:

response_matrix.matrix[:,len(response_matrix.input_names)//2:] *= -1

or if you pass an empty vcorr array to calculation of the ideal ORM, which you can then use with a gain=-1

It is worse. Now the 2 planes are failling....
pyaml is not using KickAngle whic is definitely wrong ;)
I can try to recompute the ORM and trick set strength.

[kparasch]

    if (KickAngle) {   /* Convert corrector component to polynomial coefficients */
        B[0] -= sin(KickAngle[0])/le;
        A[0] += sin(KickAngle[1])/le;
    }

On the difference between PolynomB[0] and KickAngle[0], I am tempted to think that we should only be concerned about the sign. When the approximation B[0] = - sin(KickAngle[0])/le = -KickAngle[0]/le stops being valid, then there are bigger issues with the model and you need to use the exact Hamiltonian where KickAngle is no longer an angle. Moreover, when measuring a magnet you make a correspondence between B[0] and the magnet's current, so I would argue using PolynomB[0] in pyaml is better practice.

P.S. If we ever want to cover positron rings or rings that are defined in counter-clockwise frame, there will likely be another sign change that we will need to take care of.

[JeanLucPons]

Yes yes, i agree. I will add a sign in PolynomInfo as a quick fix.

[JeanLucPons]

class HCorrector(Magnet):
    """Horizontal Corrector class"""
    polynom = PolynomInfo('PolynomB',0, -1.)

    # Sets the value
    def set(self, value:float):
        self.__poly[self.__polyIdx] = value * self.sign / self.__elements[0].Length

This is not better, same issue :(
I think I'm too tired and that I need to rest.

[JeanLucPons]

Now, I'm sure there is a sign issue somewhere:

            data[corr] -= trim_list[i] * gain

And it swapped the issue.

[JeanLucPons]

Sign issue fixed in PR #84

On design:

[ubuntu20acu.pons] > python correct_orbit.py 
Creating dummy TangoControlSystem: live
R.m.s. orbit before correction H:  87.8 µm, V:  61.4 µm,
R.m.s. orbit after correction H:  1.0 µm, V:  0.8 µm,

On live:

[ubuntu20acu.pons] > python correct_orbit-jlp.py 
24 Nov% 2025, 09:23:32 | INFO | Tango control system binding for PyAML initialized with name 'live' and TANGO_HOST=ebs-simu-3:10000
R.m.s. orbit before correction H:  122.4 µm, V:  76.5 µm,
R.m.s. orbit after correction H:  1.6 µm, V:  1.4 µm,

Now it would be nice to (if not already done):

• Add dispersive orbit, RF is available from sr.design.get_rf_plant("RF").frequency.set()
• Add lamba functions for callback in between 2 orbit measure after that the steerer has been restore (to reflatten the orbit in order to compensate for steerer hysteresis, various diagnostic check, sleeping time, ...).
• Other callbacks at other step of the process can be added as well
• Add steerer sum control
• BPM and steerer weights

Thanks again @kparasch for this very good job 👍

[kparasch]

To summarize:

I have created three tuning_tools objects: OrbitResponseMatrix, Dispersion, Orbit. At the moment we need to initialize them in the script along with the element holder (e.g. sr.live or sr.design). And then we can call their functions. For example the dispersions script goes like

sr = Accelerator.load(config_path)
element_holder = sr.design

dispersion = Dispersion(
    cfg=Disp_ConfigModel(
        bpm_array_name="BPM",
        rf_plant_name="RF",
        frequency_delta=10,
    ),
    element_holder=element_holder,
)

dispersion.measure()

We definitely should aim for something like:

sr.design.dispersion.measure()

but I would like it the subject of a separate PR, since it can be an independent PR.

For now I have included these examples and tests:

examples/ESRF_ORM_example/correct_orbit.py
examples/ESRF_ORM_example/measure_dispersion.py
examples/ESRF_ORM_example/measure_ideal_ORM_and_disp.py
examples/ESRF_ORM_example/measure_ideal_ORM.py
examples/ESRF_ORM_example/measure_reduced_ORM.py
examples/ESRF_ORM_example/esrf_orm_example.py

and also a very small set of unit tests:

test_tuning_dispersion.py
test_tuning_orbit_correction.py
test_tuning_orm.py

and a big file used during the orbit correction test:

config/ideal_orm_disp.json

pySC is included as a submodule at this moment. I initially wanted to keep it like this so that the pySC included in pyAML can be independent of any external pySC installation. Seeing how much trouble I have had to make the github workflow run, perhaps it is simpler to just have it as a dependency as Jean-Luc also suggested to me.

You can try out the tests yourself but you need to initialize the submodule:

git clone git@github.com:python-accelerator-middle-layer/pyaml.git
cd pyaml
git submodule update --init --recursive
pip install .

The tests are succeeding locally after putting the git submodule update --init --recursive command in the workflow, but I would need to add some pySC dependencies (h5py and matplotlib) to have them run remotely in the github action. Since we plan to at some point provide plotting features and h5py data formats, I think there shouldn't be a problem including these dependencies already.

I think this PR is in a good state to be merged before it becomes too big and we can improve the functionality in later PRs.

[gubaidulinvadim]

@kparasch tests are failing, is this intended functionality :) ?

[kparasch]

@gubaidulinvadim Tests should be ok now

[gubaidulinvadim]

Looks good! I hope it is compatible with the latest Jean-Luc's latest refurbishment of control system access. I have a question, where are the wait times/sleeps handled for the measurements? Is it done on the level of pySC? In general, we should be using set_and_wait of pyAML but it is not implemented yet, I think.

[kparasch]

The control access is very minimal actually in this PR, so if it is not compatible with Jean-Luc's refurbishment, it will not be difficult to adapt.

pySC does not wait for anything :) The only place where the set is actually done is on the pySC_interface.py file. Now it's using the set of pyAML but indeed we should go for 'set_and_wait' when it is implemented. Personally I don't see a good way to avoid putting waiting times. I think we should review this topic more at some point. Even after the 'set_and_wait' function has finished (based on power supply readback value I assume) we will have to at least handle decay of eddy current fields and other transients, as well as polling rate and integration time of electronics for BPMs.

In the ESRF tests, we put some waiting times as attributes in the equivalent pySC_interface which we would tweak.

[JeanLucPons]

So should we merge now and then we update tuning_tools classes to pyaml standard (attachment, PYAMLCLASS, ...) ?
Or should we work here ?

[kparasch]

I would prefer to merge it and continue with a new PR. I think it will also be easier for you to review the changes. If you prefer to continue from here I can also do that.

[JeanLucPons]

Nice work.
Thanks again Kostas !

[JeanLucPons]

@kparasch
Sorry but i missed the ideal_orm_disp.json which is in the tests/config.
Is it necessary or could it be removed ?

[kparasch]

It is used in the tests where I do an actual orbit correction. I could also generate it on the spot but it will increase the time for tests quite a lot.

[kparasch]

I would also like to include in the configuration of the orbit correction actually.

[JeanLucPons]

I'm currently working on that topic.
The json file inside the config directory make me fall in a trap :)
The fileloader tried to expand it and it produces an error:

 File "/operation/common/miniconda/envs/jlp-py312/lib/python3.12/site-packages/pyaml/configuration/factory.py", line 85, in build_object
    raise PyAMLConfigException(
pyaml.common.exception.PyAMLConfigException: No type specified for <class 'dict'>:{'matrix': [[6.729972986757497, -1.3761835949270493, -3.401549294273719, -2.483733598204853, 2.5083614871650197, 1.3
086088626362293, 3.288580877680729, 2.4579648499386346, 1.6587052665419806, -2.4895973574776478, -1.7498511136493395, -1.9890134987248003, -0.5280942625067324, -1.3344468198851498, 1.798405083371433
, 2.1151768405859968, 0.6258037278410615, -4.3473586932393875, -1.054623813946775, 3.965723476007399, 6.3083601749
...

I had to trick the fileloader using:

# Expand condition
def hasToExpand(key,value):    
    return "file" not in str(key) and isinstance(value, str) and any(
        value.endswith(suffix) for suffix in accepted_suffixes
    )

[kparasch]

Ah! Not good!
I think that actually we would want to parse it though "when pyaml is configured properly".

I would propose I make a ResponseMatrix pyaml class so that it actually parses it. Until then perhaps we can delete the file and disable the test.

[JeanLucPons]

No worries. In any case we have to define a rule to not expand a field.
I also need that to use yaml calibration curves rather than CSV.
So the rule that a field which contains "file" is not expanded is not so bad.