217 strange behavior of serialized magnets

[gupichon]

Description

This PR fixes the strange behavior observed on serialized magnets when setting either the strength or the hardware value.

The issue report showed that, in the test configuration for serialized magnets, setting a strength on the serialized object could leave the first magnet with the requested strength while the others ended up with another common value, which was confusing and inconsistent from the user point of view .

From the discussion, two points emerged:

1. The original test configuration was tricky because it grouped magnets that were not really compatible to be used in series with the selected conversion curves.
2. The serialized-strength propagation logic still needed to behave consistently once a common hardware value is derived and applied to all magnets in the series.

This PR makes the serialized magnet behavior consistent again for both directions:

• when setting a strength, the resulting common hardware value is propagated correctly to the whole series,
• when setting a hardware value, all magnets in the series remain coherent.

Related Issue

• Fixes Strange behavior of serialized magnets #217

Features/issues described there are:

• bugfix: serialized magnets now propagate set operations consistently across all magnets in the series
• bugfix: the serialized magnet test configuration was adjusted so that the magnets and conversion curves used in the test are compatible with the expected serialized behavior
• bugfix: a regression test verifies that both strength.set(...) and hardware.set(...) keep the series coherent

Changes to existing functionality

Describe the changes that had to be made to an existing functionality (if they were made)

• Serialized magnet strength handling: reworked so that the common hardware value derived from the requested serialized strength is applied consistently to the whole series
• Serialized magnet test data: updated to use an adapted curve/configuration for the test case, avoiding the misleading situation caused by an unsuitable conversion curve discussed in the issue
• Regression coverage: added/updated the serialized magnet test to explicitly check coherence after both strength and hardware writes

Testing

The following tests (compatible with pytest) cover this change:

• test_config_load in test_serialized_magnets.py, which verifies that:
  • after setting one magnet strength in the series, all strengths remain coherent,
  • after setting one hardware value in the series, all hardware values and strengths remain coherent

Verify that your checklist complies with the project

• New and existing unit tests pass locally
• Tests were added to prove that all features/changes are effective
• The code is commented where appropriate
• Any existing features are not broken (unless there is an explicit change to an existing functionality)

[JeanLucPons]

OK Thanks.
The only point that I'm happy with it that I cannot use this in tuning tools.
And looking at your code, this is incoherent:

strength = m.strengths.get() 
m.strengths.set(strength + 1e-4)

I expect:

strength = m.strength.get() # Should return the sum of all underlying strength
m.strength.set(strength + 1e-4) # Should spread the given strength to all underlying magnet according to the length

>>> print(quadForTuneDesign.strengths.get())
[-0.61758841 -0.61758841  0.79250831  0.79250831]
# should return value ~32 times bigger

[JeanLucPons]

def get_serialized_magnet(self, name: str) should return a Magnet as before

[JeanLucPons]

That true that our QF1 and QD2 does not have the same current due to our canted beamlines.
You can set the same current (or strenght) for all as you did.
Then to apply the strenght, you can do as you did (I presume) by computing the first strength, get the current and apply this current to all. But to compute this first strenght you have to:
$str_0 = \frac{str_{input} * lenght_0}{ \Sigma length_i}$

[JeanLucPons]

I add that when you want to deal with serialized magnet that are not all the same (the reality)
You only read strength and you apply or read current.
So to get the matrix it would be better to do it in hardware using:

m.hardware.set(...)

[gupichon]

That true that our QF1 and QD2 does not have the same current due to our canted beamlines. You can set the same current (or strenght) for all as you did. Then to apply the strenght, you can do as you did (I presume) by computing the first strength, get the current and apply this current to all. But to compute this first strenght you have to: $str_0 = \frac{str_{input} * lenght_0}{ \Sigma length_i}$

It is implemented in the RWSerializedStrength class in the pyaml.lattice.abstract_impl package. I will fix it tomorrow.

[gupichon]

It should be OK now, @JeanLucPons. Could you take a look? Sorry to bother you, I don’t have your level of expertise on this matter.

[JeanLucPons]

Thanks
I'll do it after lunch before the meeting.

[JeanLucPons]

sr = Accelerator.load("tests/config/sr_serialized_magnets.yaml", use_fast_loader=True, ignore_external=True)
m = sr.design.get_serialized_magnet("QF1A")
print(m.strength.get())

Traceback (most recent call last):
  File "/home/esrf/pons/pyaml/tests/jlp_test.py", line 21, in <module>
    print(m.strength.get())
          ^^^^^^^^^^
AttributeError: 'SerializedMagnets' object has no attribute 'strength'. Did you mean: 'strengths'?

I expect that a string has a strength attribute not strengths (same for harware), otherwise no way to use it in tuning tools.
A string has to be consider has a big single magnet, if you want to get the strength on a element, then you should access individual elements.

[JeanLucPons]

Could you also update comments in serialized_manget.py and remove unexpected "combined function magnet" ?

[gupichon]

If it was your last remarks, it should be ok now.

[JeanLucPons]

>>> print(m.get_magnets())
[]
>>> print(m.get_nb_magnets())
31

?

[gupichon]

>>> print(m.get_magnets())
[]
>>> print(m.get_nb_magnets())
31

?

Corrected

[JeanLucPons]

sr = Accelerator.load("tests/config/sr_serialized_magnets.yaml", use_fast_loader=True, ignore_external=True)
m:SerializedMagnets = sr.design.get_serialized_magnet("QF1A")
print(m.strength.get())
print(m.get_nb_magnets())
print(m.get_magnets()[0].strength.get())
print(m.get_magnets()[1].strength.get())

outputs:

24.651047425284716  # Correct
31                  # Correct
24.651047425284716  # not correct
24.651047425284716  # not correct

[JeanLucPons]

With the same model for all QF1A magnets in the string, I expect:

def get_strenghts_from_lattice(sr,sm):
    ring = sr.design.get_lattice()
    strs = []
    for m in sm.get_magnets():
        elt = ring.get_elements(f"{m.get_name()}")[0]
        strs.append(elt.K*elt.Length)
    return strs

sr = Accelerator.load("tests/config/sr_serialized_magnets.yaml", use_fast_loader=True, ignore_external=True)
m:SerializedMagnets = sr.design.get_serialized_magnet("QF1A")
print(m.strength.get())
print(m.get_nb_magnets())
# Strengths that should be returned using [m.strength.get() for m in m.get_mangets()]
print(get_strenghts_from_lattice(sr,m))  
m.strength.set(24.0)
print(get_strenghts_from_lattice(sr,m)) 

24.651047425284716
31
[0.7925083076304686, 0.7925083076304686, 0.7925083076304686, 0.7922118226646928, 0.7925083076304686, 0.7925083076304686, 0.7925083076304686, 0.7925083076304686, 0.7925083076304686, 0.7925083076304686, 0.8172956386178192, 0.8257533472095736, 0.7925083076304686, 0.7925083076304686, 0.7925083076304686, 0.7922117043717326, 0.7925083076304686, 0.7925083076304686, 0.7925083076304686, 0.7925083076304686, 0.7925083076304686, 0.7922117785682908, 0.7925083076304686, 0.7922116233599168, 0.7925083076304686, 0.8195450222441725, 0.7925083076304686, 0.7922118961610275, 0.7925083076304686, 0.792211824217169, 0.7925083076304686]
[0.7741935483870964, 0.7741935483870964, 0.7741935483870964, 0.7741935483870964, 0.7741935483870964, 0.7741935483870964, 0.7741935483870964, 0.7741935483870964, 0.7741935483870964, 0.7741935483870964, 0.7741935483870964, 0.7741935483870964, 0.7741935483870964, 0.7741935483870964, 0.7741935483870964, 0.7741935483870964, 0.7741935483870964, 0.7741935483870964, 0.7741935483870964, 0.7741935483870964, 0.7741935483870964, 0.7741935483870964, 0.7741935483870964, 0.7741935483870964, 0.7741935483870964, 0.7741935483870964, 0.7741935483870964, 0.7741935483870964, 0.7741935483870964, 0.7741935483870964, 0.7741935483870964]

[gupichon]

I added your code as a unit test. I'm working on it.

[gupichon]

It should be ok now. There is now a test with the tune and a test with your example.

[JeanLucPons]

I test it ASAP.
We have the machine restart today and I have few other things to complete