Address the dipole kick through KickAngle where the pass method uses it by thellert · Pull Request #470 · python-accelerator-middle-layer/pyaml

thellert · 2026-10-10T18:49:00Z

Description

AT keeps the dipole kick of an element in the polynom and in KickAngle; the simulator's magnet accessors read and write the polynom only. As a result a strength set on a CorrectorPass element never reaches the beam, and a KickAngle carried by a multipole element is not read and is doubled by a write. Details, measurements and a script are in the issue.

Related Issue

Fixes Dipole kick held in KickAngle is not seen by the magnet accessors #471

Features/issues described there are:

bugfix: one small view class in pyaml/lattice/abstract_impl.py (_KickAngleCoefficients) presents the dipole strength of an element that carries a KickAngle as a single polynom coefficient. The scalar and array accessors use it wherever they looked up the polynom, so their arithmetic is unchanged. This keeps the fix in one place instead of branching in each accessor.

Changes to existing functionality

CorrectorPass elements: the strength is written to and read from KickAngle (PolynomB[0] * L = -KickAngle[0], PolynomA[0] * L = KickAngle[1]), because that pass method ignores the polynoms. A polynom the element carries is kept in step, so the existing accessor tests hold unchanged.
at.Corrector elements defined without PolynomA/B now work; building the accessor raised AttributeError.
Multipole elements that carry a KickAngle (ThinMPolePass, StrMPole*, BndMPole*, ExactMultipole*): the strength reads polynom + KickAngle; a write stores the value in the polynom and resets that plane's KickAngle, so it is not applied twice. This changes an attribute of the user's lattice on write — the alternative is to keep KickAngle and store the difference in the polynom; say which you prefer.
Elements without a KickAngle, or whose pass method ignores it (QuadLinearPass, DriftPass), are untouched.

Testing

The following tests (compatible with pytest) were added to tests/lattice/test_magnet_accessors.py; each compares the closed orbit of a small ring against a KickAngle reference:

test_corrector_pass_strength_moves_the_orbit — thin and thick CorrectorPass, both planes
test_corrector_pass_without_polynoms_is_supported
test_corrector_pass_reads_the_kick_of_the_lattice
test_corrector_pass_combined_function_strengths_move_the_orbit — strength and hardware arrays
test_split_corrector_pass_shares_the_kick_by_length
test_multipole_corrector_agrees_with_corrector_pass
test_multipole_kick_angle_is_part_of_the_strength — thin multipole, multipole, dipole
test_kick_angle_ignored_by_the_pass_method_is_not_read

The orbit tests fail on main and pass here.

Verify that your checklist complies with the project

New and existing unit tests pass locally — 467 passed, 6 skipped, 1 failed: tests/tuning_tools/test_tune_hardware.py::test_tune (4.1e-8 against a 1e-8 tolerance), which fails with the same value on unmodified main on my machine (macOS, Python 3.12, AT 0.8.0)
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)

CorrectorPass integrates KickAngle and ignores PolynomA/PolynomB, so a strength set on an at.Corrector element round-tripped through the accessor but never moved the orbit. The simulator accessors now address the dipole component of such an element through KickAngle, keeping a polynom the element carries in step with it. Correctors defined without polynoms are supported, and a kick already present in the lattice is read back. Tests compare the closed orbit against a KickAngle reference.

The multipole pass methods add KickAngle to the polynom. An element that carries one read back only its polynom part, and a write added to the kick instead of replacing it. The accessors now read the sum and, on a write, store the value in the polynom and reset the KickAngle component. A KickAngle on a pass method that ignores it is not read.

gubaidulinvadim · 2026-10-12T13:28:49Z

@thellert Thanks for the contribution. Normally, we've discussed limiting pyAML elements to PolynomA/PolynomB, no kick angle, as the maintaining lab of AT is not using KickAngles much. But I think now there's enough demand to also have KickAngle in pyAML. So, this is a good addition.

gubaidulinvadim · 2026-10-12T18:57:38Z

I think here you need to test also if it works with a combined function magnet, having a sextupole with dipolar correctors as KickAngles. I think @gupichon did something similar a while ago, but maybe it was never merged.

JeanLucPons · 2026-10-13T06:30:47Z

Thank for the contribution.
Yes theoretically, we should use H or VCorrector.angle.set() which do the angle kick.
The KickAncgle of AT is a bit wrong.
As it is done in the PR I'm affraid that if you use angle.set() it will be worse.
So either we support the CorrectorPass as done is this PR and we remove angle or we forget KickAngle.

gubaidulinvadim · 2026-10-16T15:04:57Z

@thellert Thanks again for the contribution. We've discussed in the maintainer meeting that KickAngle can be implemented and allow a bridge to CorrectorPass to cover the use case. I will try to review this net week (and @JeanLucPons if he has time [not guaranteed] can guide you).

gubaidulinvadim

For me the PR looks mostly OK. I will have a closer look on Monday. The one thing that is strange is that one test fails for you. This is definitely not normal. Could you please make an issue detailing why test_tune_hardware fails for you on the main branch @thellert ?

gubaidulinvadim · 2026-10-16T16:15:08Z

+    AT holds the dipole kick of an element in two places. ``CorrectorPass`` integrates ``KickAngle`` only and ignores
+    ``PolynomA``/``PolynomB``; the multipole pass methods integrate the polynom *and* add ``KickAngle`` to it. This


For now, we would be happy to even have only KickAngle for CorrectorPass without integration. But if it's already done, it's good.

gubaidulinvadim · 2026-10-16T16:16:28Z

+
+    AT holds the dipole kick of an element in two places. ``CorrectorPass`` integrates ``KickAngle`` only and ignores
+    ``PolynomA``/``PolynomB``; the multipole pass methods integrate the polynom *and* add ``KickAngle`` to it. This
+    view exposes the total at index 0 with the polynom convention (``PolynomB[0] * L = -KickAngle[0]``,


For me, it's not immediately clear what "index 0" means in a docstring here

GamelinAl

Some pass method seem to be missing.
Otherwise, it's good for me

GamelinAl · 2026-10-19T07:36:02Z

+def _applies_kick_angle(element: at.Element) -> bool:
+    """
+    Return True when the pass method of ``element`` applies its ``KickAngle``.
+
+    That is ``CorrectorPass`` and the multipole integrators (``StrMPole*``, ``BndMPole*``, ``ThinMPolePass``,
+    ``ExactMultipole*``). Other pass methods ignore the attribute.
+    """
+    pass_method = element.PassMethod
+    return pass_method == "CorrectorPass" or "MPole" in pass_method or "Multipole" in pass_method


The list of pass methods is incomplete, those are missing:

ExactSectorBendPass

ExactRectangularBendPass and ExactRectBendPass

the *Rad variants of all three

the legacy StrOldRadPass / BndOldRadPass

kparasch · 2026-10-20T08:34:16Z

FYI. In a future pyAT release, KickAngle is likely going to be derived from PolynomB, PolynomA and will no longer be independent, see atcollab/at#1161.

kparasch · 2026-10-20T08:59:42Z

For me the PR looks mostly OK. I will have a closer look on Monday. The one thing that is strange is that one test fails for you. This is definitely not normal. Could you please make an issue detailing why test_tune_hardware fails for you on the main branch @thellert ?

@gubaidulinvadim
test_tune_hardware has always been failing on my machine too (macOS). I assumed it comes from the compilation of AT on different machines, but it takes a lot of effort and different computers to properly study this.

gubaidulinvadim · 2026-10-20T09:22:09Z

For me the PR looks mostly OK. I will have a closer look on Monday. The one thing that is strange is that one test fails for you. This is definitely not normal. Could you please make an issue detailing why test_tune_hardware fails for you on the main branch @thellert ?

@gubaidulinvadim test_tune_hardware has always been failing on my machine too (macOS). I assumed it comes from the compilation of AT on different machines, but it takes a lot of effort and different computers to properly study this.

Hm, then I suspect that it fails on all macOS machines. @thellert @kparasch an issue should be made on AT side then.

FYI. In a future pyAT release, KickAngle is likely going to be derived from PolynomB, PolynomA and will no longer be independent, see atcollab/at#1161.

That's good news, but the next release is in ~6 months. We could use @thellert solution in the meantime.

kparasch · 2026-10-20T10:29:45Z

FYI. In a future pyAT release, KickAngle is likely going to be derived from PolynomB, PolynomA and will no longer be independent, see atcollab/at#1161.

That's good news, but the next release is in ~6 months. We could use @thellert solution in the meantime.

Absolutely. I would propose a test is included here to catch this and make sure we don't forget about it whenever/if this change happens and is released in AT.

swhite2401 · 2026-10-20T10:35:19Z

Absolutely. I would propose a test is included here to catch this and make sure we don't forget about it whenever/if this change happens and is released in AT.

This is already done in AT and is part of the release 0.9.0, see: atcollab/at#1158
for details.

JeanLucPons · 2026-10-20T10:41:37Z

So we can just cancel this PR ?

kparasch · 2026-10-20T10:42:31Z

@swhite2401 I see HKick and VKick introduced, but I don't see KickAngle yet as a property derived from PolynomB, PolynomA.

JeanLucPons · 2026-10-20T10:47:47Z

In any case, in pyAML angles are accessible through [H,V]Corrector.angle.set() and loads PolynomA/B.
For me the purpose of this PR is unclear except for CorrectorPass that does not have polynoms.

kparasch · 2026-10-20T10:50:49Z

In any case, in pyAML angles are accessible through [H,V]Corrector.angle.set() and loads PolynomA/B. For me the purpose of this PR is unclear except for CorrectorPass that does not have polynoms.

It also handles cases where an existing lattice has a Multipole with PolynomB[0] != - KickAngle[0]. It is also unclear to me what will happen to such lattices in AT.

Edit: In fact with PolynomB[0] != 0 and KickAngle[0] != 0

JeanLucPons · 2026-10-20T10:54:53Z

As I understand, I think KickAngle is expected to be 0 in lattice file.
KickAngle is rather meant for correction purpose. PolynomA/B[0] used in integrators is the sum of - KickAngle[0], KickAngle[1] and existing PolynomA/B[0].

gubaidulinvadim · 2026-10-20T11:02:08Z

In any case, in pyAML angles are accessible through [H,V]Corrector.angle.set() and loads PolynomA/B. For me the purpose of this PR is unclear except for CorrectorPass that does not have polynoms.

Yes, the purpose is mainly to support CorrectorPass, which is present in many lattices and is used at many facilities for simulated commissioning studies.

swhite2401 · 2026-10-20T11:06:37Z

I would strongly suggest to use HKick and VKick, this is the simplest solution for you, it is always present in any passmethod and allows to use any magnet as corrector.

KickAngle still has legacy behavior, it is added on top of what is in PolynomA/B -> there are by the way different things, one is the deflection angle the other the polynomial expansion of the field (be careful with sign!)

JeanLucPons · 2026-10-20T11:07:25Z

So I don't understand this:

def _applies_kick_angle(element: at.Element) -> bool:
    """
    Return True when the pass method of ``element`` applies its ``KickAngle``.

    That is ``CorrectorPass`` and the multipole integrators (``StrMPole*``, ``BndMPole*``, ``ThinMPolePass``,
    ``ExactMultipole*``). Other pass methods ignore the attribute.
    """
    pass_method = element.PassMethod
    return pass_method == "CorrectorPass" or "MPole" in pass_method or "Multipole" in pass_method

If I understand well this PR affect all the passmethod that match.

kparasch · 2026-10-20T11:08:21Z

As I understand, I think KickAngle is expected to be 0 in lattice file. KickAngle is rather meant for correction purpose.

There are cases where the design can have non-zero kick angles, e.g. canting in light sources with correctors, modeling of shifted quadrupoles (without touching dx), colliders for crossing angles between beams, e.t.c.
I highly dislike the degeneracy of KickAngle, but this is just a personal preference.

swhite2401 · 2026-10-20T11:15:54Z

I highly dislike the degeneracy of KickAngle, but this is just a personal preference.

Exactly and this is why we suggest to use HKick and VKick in the future, this is much cleaner, KickAngle will only be kept for backward compatibility and eventually be derived from the same quatity

JeanLucPons · 2026-10-20T12:41:58Z

OK so, this PR should:

remove Corrector.angle.set()/get()
remove corrector.py that does more or less the same things as in AT magnet_elements.py
remove sign convention and _sign field in lattice/abstract_impl, PolynomInfo, constant.py
map Corrector.strength.set() to HKick and VKIck for all magnets (AT handles KickAngle when needed)
- put "HKick" and "VKick" in PolynomInfo for H/V Corrector
- Either add integrated flag for HKick and VKIck or switch all to integrated polynomials in AT (HKick,VKick,K1nL,K1sL,etc...)

JeanLucPons

Changes mentioned above

thellert added 2 commits October 10, 2026 09:14

thellert changed the title ~~Write corrector strength to KickAngle for CorrectorPass elements~~ Address the dipole kick through KickAngle where the pass method uses it Oct 10, 2026

thellert mentioned this pull request Oct 10, 2026

Dipole kick held in KickAngle is not seen by the magnet accessors #471

Open

gubaidulinvadim requested review from GamelinAl, JeanLucPons and gupichon October 12, 2026 13:24

gubaidulinvadim assigned thellert Oct 12, 2026

gubaidulinvadim self-requested a review October 12, 2026 13:25

gubaidulinvadim added the enhancement New feature or request label Oct 12, 2026

gubaidulinvadim requested changes Oct 16, 2026

View reviewed changes

GamelinAl requested changes Oct 19, 2026

View reviewed changes

JeanLucPons requested changes Oct 20, 2026

View reviewed changes

		AT holds the dipole kick of an element in two places. ``CorrectorPass`` integrates ``KickAngle`` only and ignores
		``PolynomA``/``PolynomB``; the multipole pass methods integrate the polynom and add ``KickAngle`` to it. This

Conversation

thellert commented Oct 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Related Issue

Changes to existing functionality

Testing

Verify that your checklist complies with the project

Uh oh!

gubaidulinvadim commented Oct 12, 2026

Uh oh!

gubaidulinvadim commented Oct 12, 2026

Uh oh!

JeanLucPons commented Oct 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

gubaidulinvadim commented Oct 16, 2026

Uh oh!

gubaidulinvadim left a comment

Choose a reason for hiding this comment

Uh oh!

gubaidulinvadim Oct 16, 2026

Choose a reason for hiding this comment

Uh oh!

gubaidulinvadim Oct 16, 2026

Choose a reason for hiding this comment

Uh oh!

GamelinAl left a comment

Choose a reason for hiding this comment

Uh oh!

GamelinAl Oct 19, 2026

Choose a reason for hiding this comment

Uh oh!

kparasch commented Oct 20, 2026

Uh oh!

kparasch commented Oct 20, 2026

Uh oh!

gubaidulinvadim commented Oct 20, 2026

Uh oh!

kparasch commented Oct 20, 2026

Uh oh!

swhite2401 commented Oct 20, 2026

Uh oh!

JeanLucPons commented Oct 20, 2026

Uh oh!

kparasch commented Oct 20, 2026

Uh oh!

JeanLucPons commented Oct 20, 2026

Uh oh!

kparasch commented Oct 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

JeanLucPons commented Oct 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

gubaidulinvadim commented Oct 20, 2026

Uh oh!

swhite2401 commented Oct 20, 2026

Uh oh!

JeanLucPons commented Oct 20, 2026

Uh oh!

kparasch commented Oct 20, 2026

Uh oh!

swhite2401 commented Oct 20, 2026

Uh oh!

JeanLucPons commented Oct 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

JeanLucPons left a comment

Choose a reason for hiding this comment

Uh oh!

This branch has not been deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

thellert commented Oct 10, 2026 •

edited

Loading

JeanLucPons commented Oct 13, 2026 •

edited

Loading

kparasch commented Oct 20, 2026 •

edited

Loading

JeanLucPons commented Oct 20, 2026 •

edited

Loading

JeanLucPons commented Oct 20, 2026 •

edited

Loading