Skip to content

Feature: CFM refurbishment and magnet polynomial linking #299

Open
1 of 1 issue completed
Open

Description

@gupichon

Description, motivation and use case
So far, CFM mapping keys such as B0, A0, B1, etc. are hardcoded to specific magnet classes. Each class defines its own PolynomInfo, which links the virtual magnet to a fixed AT polynomial attribute and coefficient index.
This makes the AT polynomial linking rigid. For example, pyAML cannot directly target KickAngle, and the current mapping does not support polynomial coefficient indexes above 3.
This limitation mainly affects simulator/design usage, where pyAML must read from and write to AT lattice attributes. Live control-system access remains driven by the configured model/backend/catalogue devices.

Proposed solution
Parse the CFM mapping key to retrieve the target AT attribute and index to write to. The mapping should support at least PolynomA, PolynomB, and KickAngle targets.
The existing H and V correctors should be updated by implementing a RWCorrectorStrength mirroring the RWCorrectorAngle so they can target either the current polynomial coefficient representation or the AT KickAngle attribute while preserving the same public behavior.
To manage indexes > 3, a new generic magnet type should be created and all other magnet classes should inherit from it to avoid code duplication. They would become empty classes, just for typing.

class GenericMagnetConfigModel(MagnetConfigModel):
    polynom_name: str
    polynom_index: int | str  # For KickAngle, H/V aliases could map to 0/1.


class GenericMagnet(Magnet):
    def __init__(self, cfg: GenericMagnetConfigModel):
        super().__init__(cfg.name, cfg.model if hasattr(cfg, "model") else None)

        if isinstance(cfg.polynom_index, str):
            if cfg.polynom_index in ["H", "V"]:
                index = 0 if cfg.polynom_index == "H" else 1
            else:
                raise PyAMLException(f"Unsupported polynomial index alias: {cfg.polynom_index}")
        else:
            index = cfg.polynom_index

        self.polynom = PolynomInfo(cfg.polynom_name, index)
        self._cfg = cfg

        
class SextupoleConfigModel(MagnetConfigModel):
    """Configuration model for Sextupole magnet."""

    ...


class Sextupole(GenericMagnet):
    def __init__(self, cfg: SextupoleConfigModel):
        super().__init__(
            GenericMagnetConfigModel(
                name=cfg.name,
                model=cfg.model,
                polynom_name="PolynomB",
                polynom_index=2,
            )
        )

Also, the duplication of the multipole list in cfm_magnet and its model should be removed. The units can be moved to the CFM and it will initialize the model with its data (multipole list)

The existing configuration files will have to be updated for the CFM part only.
The final mapping syntax is intentionally left open and should be defined with the people involved in this topic. The examples below are proposals meant to support the discussion, not final API decisions.

Note: the usage of indexes above 3 may only be used for development purposes on the design part. Nevertheless, it should be possible.

Describe alternatives you've considered
Keep the current implementation and just add KickAngleH and KickAngleV in the mapping as aliases to HCorrector and VCorrector with the new RWCorrectorStrength.
It does not cover the case of indexes > 3 and it is not as flexible.

Another way to achieve it would be to create a small grammar similar to the one used to point indexes for backends devices: PolynomA@0, KickAngle@1, ...
So the PyAt attribute is also retrieved from user input. It adds some complexity though, and it is less readable.

Example

- type: pyaml.magnet.cfm_magnet
  name: SJ2A-C04
  mapping:
    - [KickAngleH, SJ2A-C04-H]
    - [KickAngleV, SJ2A-C04-V]
  units: [rad, rad]
  model:
    type: pyaml.magnet.identity_cfm_model
    physics:
        - srmag/m-sf2/c04-a/Strength_H
        - srmag/m-sf2/c04-a/Strength_V
- type: pyaml.magnet.cfm_magnet
  name: SH1_COR_001
  mapping:
    - [B2, SH1_COR_001.sextupole]
    - [B0, SH1_COR_001.hcorrector]
    - [A0, SH1_COR_001.vcorrector]
    - [A4, SH1_COR_001.4]
    - [A5, SH1_COR_001.5]
  units: [1/m**2, '1', '1', '1', '1']
  model:
    type: pyaml.magnet.identity_cfm_model
    physics:
    - AN01-AR/EM-SX/SHF.01/strength
    - AN01-AR/EM-COR/SHF.01-CDLH.01/strength
    - AN01-AR/EM-COR/SHF.01-CDLV.01/strength
    - AN01-AR/EM-COR/SHF.01-CDL4.01/strength
    - AN01-AR/EM-COR/SHF.01-CDL5.01/strength
- type: pyaml.magnet.generic_magnet
  name: B_011
  polynom_name: PolynomB
  polynom_index: 0
  model:
    type: pyaml.magnet.identity_model
    unit: 1
    physics: AN06-AR/EM-DIP/B.01

Additional context
The CFM refurbishment and the polynomial mapping can be treated into two sub-issues. Since the two subjects are strongly linked together, it can be done in one. This still needs to be defined.

Checklist

  • I've assigned this issue to a project
  • I've @-mentioned relevant people

Activity

  1. gupichon commented on Jun 25, 2026

    @gupichon
    MemberAuthor

    @JeanLucPons, I'd like your feedback on this.

  2. kparasch commented on Jun 25, 2026

    @kparasch
    Member

    Personally, I am against using KickAngle in pyaml.

    During tracking in AT, KickAngle is used to modify PolynomB[0] and PolynomA[0] as in:

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

    PolynomB and PolynomA are directly proportional to magnetic field strengths (multipolar coefficients), while KickAngle becomes technically a non-linear function of the strengths.

    A common exception to this is when elements are using the "CorrectorPass" integrator. In that case, PolynomB and PolynomA have no effect, but the integrator now does the equivalent of

            B[0] = - KickAngle[0]/le;
            A[0] = KickAngle[1]/le;
    

    In practice, sin(KickAngle) will always be approximately equal to KickAngle.

    Making a distinction between KickAngle and PolynomB[0]/PolynomA[0] would give too many complications in my view for no actual gain if we want to stay proper to what AT does internally.

    I would propose that we keep using PolynomInfo and if, on the simulator's side the element uses CorrectorPass, then B[0] would be mapped internally to -KickAngle[0] and A[0] to +KickAngle[1].

  3. gupichon commented on Jun 25, 2026

    @gupichon
    MemberAuthor

    @GamelinAl, @gubaidulinvadim I'll let you give your feedback on @kparasch's comment. I don't have a strong opinion on the matter, except perhaps that users should be able to map their devices to the lattice as they wish. Using KickAngle should be possible but not mandatory, and the core behavior would remain unchanged.

    That is completely open to debate, and I will back the qualified majority (which I am not part of!)

  4. gubaidulinvadim commented on Jun 25, 2026

    @gubaidulinvadim
    Member

    I think what @kparasch proposed is good. There's a different practice in different facilities. ESRF never uses the corrector pass, for example. SOLEIL uses a lot of the corrector pass. For me personally, it's ok to use PolynomA/PolynomB everywhere and just map corrector kick angles to polynoms. But for other colleagues, I do not know yet. @TeresiaOlsson should also chip in the HZB opinion.

  5. TeresiaOlsson commented on Jun 25, 2026

    @TeresiaOlsson
    Member

    At HZB we use the KickAngle field in pyAT for all our correctors since they are extra windings on other magnets. This has worked well for us so far but if pyAML implements it in some other way we can give it up. We only use it because pyAT doesn't have any other way to separate the component coming from the main coil and the corrector windings.

    The only important part for us is that we can distinguish what the corrector strength is compared to the other components. What probably is different for us compared to other machines is that BESSY II has a dipole corrector in our main dipoles. Other than that our correctors are extra windings on the sextupoles.

  6. TeresiaOlsson commented on Jun 25, 2026

    @TeresiaOlsson
    Member

    I think it would also be useful to distinguish between combined function magnets and single function magnets with a corrector winding on top. It would be very strange for our users to have to configure a combined function magnet for our magnets just because of the corrector.

  7. TeresiaOlsson commented on Jun 25, 2026

    @TeresiaOlsson
    Member

    I really like @gupichon's suggestion to make a generic magnet type and have all other magnet classes inherit from it. That I think fits well with the users' view that every magnet is a multipole and then there are subclasses of it based on what combination of components your physical magnet has.

  8. GamelinAl commented on Jun 25, 2026

    @GamelinAl
    Member

    I agree that it's not strictly needed to have KickAngle in pyAML, it's something which is very pyAT specific (and maybe a confusing feature in fact). So for me it's ok to convert KickAngle to A0/B0.

    The only point to watch for is that having a pyAT lattice with a pyAT Corrector element should not be a blocking element from using pyAML.

    That also mean that, for lattice with pyAT Corrector element we accept to have small differences between pyAML simulator output and pyAT (and a virtual accelerator based on pyAT). @kparasch is this correct?

  9. gupichon commented on Jun 25, 2026

    @gupichon
    MemberAuthor

    So @kparasch, if I understand your proposal correctly, we should always use A/B[0] in pyAML, and when mapping to pyAT, we would do something like this:

    if corrector_pass:
      pyat_elem.KickAngle[0] = -given__pyaml_B[0]
      pyat_elem.KickAngle[1] = given__pyaml_A[0]
    else:
      pyat_elem.PolynomB[0] = given__pyaml_B[0]
      pyat_elem.PolynomA[1] = given__pyaml_A[0]

    @TeresiaOlsson, you are never forced to create a combined-function magnet if you have separate power supplies. You can just declare them as separate elements. @JeanLucPons will correct me if I'm wrong, but CFMs are useful when you don't have a 1-to-1 match between the physical magnets and the power supplies.

  10. GamelinAl commented on Jun 25, 2026

    @GamelinAl
    Member

    @TeresiaOlsson I don't agree with your comment regarding "distinguish between combined function magnets and single function magnets with a corrector winding on top." Any magnet which has multiple function is a combined function magnet.

    The full prupose of the CFM class is to deal with this. And to be able to adress the different component independantly by passing a model which can deal with cross-talk and this type of effect.

  11. TeresiaOlsson commented on Jun 25, 2026

    @TeresiaOlsson
    Member

    So @kparasch, if I understand your proposal correctly, we should always use A/B[0] in pyAML, and when mapping to pyAT, we would do something like this:

    if corrector_pass:
    pyat_elem.KickAngle[0] = -given__pyaml_B[0]
    pyat_elem.KickAngle[1] = given__pyaml_A[0]
    else:
    pyat_elem.PolynomB[0] = given__pyaml_B[0]
    pyat_elem.PolynomA[1] = given__pyaml_A[0]

    @TeresiaOlsson, you are never forced to create a combined-function magnet if you have separate power supplies. You can just declare them as separate elements. @JeanLucPons will correct me if I'm wrong, but CFMs are useful when you don't have a 1-to-1 match between the physical magnets and the power supplies.

    Yes, that's how the BESSY II example currently is configured. But if I declare one dipole magnet and one corrector as separate elements which should map to the same polynom in the lattice pyAML somehow internally needs to know how much the contribution of each part is. Otherwise the current for the two different power supplies can't be calculated.

  12. 9 remaining items

  13. JeanLucPons commented on Jun 25, 2026

    @JeanLucPons
    Member

    KickAngle is wrong (unless it has been fix in AT?)
    This has already been discussed several times and good calculation is done in pyAML using amagnet.angle.set().
    However as Kostas said it is better to use PolynamA[0] B[0] directly as it is done today rather than using AT KickAngle.
    If you want to give access to KickAngle please create other classes and do not change actual behavior
    Note that is does not concern only combined function magnet.
    The stop a (skew) octupole has also been discussed. It was larger at the beginning.

  14. JeanLucPons commented on Jun 25, 2026

    @JeanLucPons
    Member

    I add that if you want to deal with errors and need access to higher polynomial order, this is an other topic rather link to error and SC.

  15. removed their assignment
    on Jun 25, 2026
  16. TeresiaOlsson commented on Jun 25, 2026

    @TeresiaOlsson
    Member

    Can you somehow configure which components that should be serialized and the name for each component?

    Actually, you can declare serialized magnets that are powered by the same power supply. The serialized magnet feature in pyAML is designed to transcribe that reality into the designpart. In your example, you should declare the correctors independently.

    But then I would lose the possibility to add the coupling that can exist if powering both the main coil and the corrector coil like @kparasch mentioned, if I have understood it correctly?

    Yes

    Then this is probably something to think about further in the future. I think it's quite standard for many machines to have a serialized main magnet with extra windings on top and for those you have individual power supplies. Feels like many users potentially could start with configuring their correctors independently and as they work on improving their machine model they discover that the coupling actually isn't negligible and they want to include it without changing how the users access different hardware components. I can see us doing that for BESSY III.

  17. TeresiaOlsson commented on Jun 25, 2026

    @TeresiaOlsson
    Member

    KickAngle is wrong (unless it has been fix in AT?) This has already been discussed several times and good calculation is done in pyAML using amagnet.angle.set(). However as Kostas said it is better to use PolynamA[0] B[0] directly as it is done today rather than using AT KickAngle. If you want to give access to KickAngle please create other classes and do not change actual behavior Note that is does not concern only combined function magnet. The stop a (skew) octupole has also been discussed. It was larger at the beginning.

    The KickAngle in AT is wrong? Why is it exposed to the users then? We use it and had no idea that it is wrong.

  18. JeanLucPons commented on Jun 25, 2026

    @JeanLucPons
    Member

    The KickAngle in AT is wrong? Why is it exposed to the users then? We use it and had no idea that it is wrong.

    Because it is a bug it happens. a sin() is used instead of a tan(). However due to small angle, the error is weak but howerver larger that doing doing nothing as done in pyaml when you use .strenght().

  19. JeanLucPons commented on Jun 25, 2026

    @JeanLucPons
    Member
  20. TeresiaOlsson commented on Jun 25, 2026

    @TeresiaOlsson
    Member

    KickAngle bug: atcollab/at#1001 (comment)

    Ah! Thank you! That was exactly what I was looking for so I can share with my users :)

  21. GamelinAl commented on Jun 25, 2026

    @GamelinAl
    Member
    1. I would say it's needed but mostly for error as pointed out by @JeanLucPons maybe it can be done later and in this context.
    2. It depends why we need it, it's not so clear anymore. I think from this discussion the point which stands out is that from KickAngle what we need is a pyAML mapping to A0/B0
    3. If the duplication is still there, then yes I would say we should try to avoid it
  22. JeanLucPons commented on Jun 25, 2026

    @JeanLucPons
    Member

    1 Yes I think this is out of scope here (already discussed in other topics)
    2 HCorrector and VCorrector already maps to A0/B0 (either via integrated strength or angle) whatever the passmethod (having poly) and not necessarily using combined function magnet. I would recommend to get rid off CorrectorPass but if you need it please create other pyaml magnet class.
    3 OK, it would also be nice to rename [A0,B0,B1,.. to the class name [Vcorrector,HCorrector ,Quadrupole,...] this should allow code simplification as well.

    - type: pyaml.magnet.cfm_magnet
      name: SH1_COR_001
      mapping:
        - [Sextupole, SH1_COR_001.sextupole]
        - [HCorrector, SH1_COR_001.hcorrector]
        - [VCorrector, SH1_COR_001.vcorrector]
    
  23. TeresiaOlsson commented on Jun 25, 2026

    @TeresiaOlsson
    Member

    Renaming A0, B0... etc I think would also help to make new users to understand the configuration.

  24. gupichon commented on Jun 26, 2026

    @gupichon
    MemberAuthor

    Ok, so to sum up:

    We should limit the changes to renaming the multipoles, removing duplications, and moving the physical units.

    Managing KickAngle for the CorrectorPass integrator should be handled later in a separate issue.
    The hardcoded names will be kept (but renamed), and polynomial coefficients above 3 will be handled differently. I didn't quite catch how, but that doesn't matter for now... Could someone explain this point?

    In the end, there's no need for generic magnets. It might be useful for the use case @TeresiaOlsson described, but it's not necessary for now.

    If it's ok with everyone, I'll create a new sub-issue for the CFM refurbishment and leave it at that.

  25. JeanLucPons commented on Jun 26, 2026

    @JeanLucPons
    Member

    For access to higher polynomial order, this concerns sr.errors (for SC or tolerance study) and not the control system. This was already discussed but I don't manage to find the link.

    I'm not really in favor of moving unit as it is the purpose of the model to convert a set of current (or voltage) to a set of strength in random unit.
    For unit conversion it has already been accepted that at the pyaml level, we will work in SI which means that if the model returns no SI unit a unit conversion will be applied. Roughly speaking at the magnet level in pyaml, the unit are fixed [rad,m-1,m-2,etc..] and for the model you have unit (the output of model calculation) and hardware_unit (the unit of the power supply).

  26. gubaidulinvadim commented on Oct 15, 2026

    @gubaidulinvadim
    Member

    @gupichon-soleil Is this still planned?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions