Skip to content

Fix H-corrector sign convention and implement scale_circumference#43

Open
thellert wants to merge 2 commits into
kparasch:mainfrom
als-apg:pr/magnet-sign-and-scale-circumference
Open

Fix H-corrector sign convention and implement scale_circumference#43
thellert wants to merge 2 commits into
kparasch:mainfrom
als-apg:pr/magnet-sign-and-scale-circumference

Conversation

@thellert

Copy link
Copy Markdown

1. Correct the sign of the integrated dipole/corrector term in Magnet.update()

Magnet.update() divides an integrated-strength link value by the magnet length
and adds it to PolynomB/PolynomA. For the order-1 normal term
(PolynomB[0], the dipole/corrector field) this did not account for AT's kick
sign convention.

In AT, the horizontal kick from PolynomB[0] is Δx' = -PolynomB[0]·L. A
positive PolynomB[0] therefore produces a negative horizontal kick, so a
positive corrector setpoint currently yields a physically negative kick — the
opposite of the intended sign. This surfaces as a sign inconsistency in
orbit-response matrices relative to the AT tracking convention.

Change: negate the integrated value for the order-1 B term only, so a
positive setpoint produces a positive physical kick:

if link.component == "B" and link.order == 1:
    value = -value

Scope notes:

  • The flip is deliberately restricted to order == 1 (PolynomB[0]). Integrated
    higher-order B strengths — e.g. an integrated quadrupole B2L
    PolynomB[1] used by the shifted-quadrupole feed-down path — must not be
    flipped, as that would invert focusing. Tests cover this explicitly.
  • Skew (A) components are never flipped; the convention flip is specific to the
    normal dipole term.
  • The state() diagnostic mirrors the same flip so its per-link printout agrees
    with the stored PolynomB[0].

Tests (tests/core/test_magnet.py):

  • integrated order-1 B is negated (B[0] == -3.0 for the worked example);
  • integrated order-2 B is not negated (B[1] == 3.0);
  • integrated A is not negated (A[0] == 3.0).

2. Implement scale_circumference in utils/sc_tools.py

scale_circumference was present only as a commented-out placeholder. This PR
provides a working implementation that scales a ring's circumference by adjusting
drift lengths only, leaving magnet lengths untouched:

def scale_circumference(ring, circ, mode='rel'):
    ...
    C = sum(elem.Length for elem in ring)
    D = sum(elem.Length for elem in ring if elem.PassMethod == 'DriftPass')
    if D == 0:
        raise ValueError('No drift elements found in ring — cannot scale circumference.')
    Dscale = 1 - (1 - circ) * C / D   # mode='rel'
    #      = 1 - (C - circ) / D       # mode='abs'
    ...
  • mode='rel' (default): circ is a multiplicative factor (e.g. 1 + 1e-6);
    the new total circumference is circ · C.
  • mode='abs': circ is the desired absolute circumference in metres.
  • The full circumference change is absorbed by the drift elements (Dscale), so
    magnet lengths are preserved; a zero-drift ring raises rather than dividing by
    zero. Modifies the ring in place and returns it for chaining.

Default-mode note for review: the placeholder carried no live default and had
no callers, so the default is a fresh choice. 'rel' is chosen because a
relative fractional stretch is the common circumference-error use (a small
multiplicative factor around 1). 'abs' remains available for setting an exact
target length.

Tests (tests/utils/test_sc_tools.py): relative/absolute modes, drift-only
scaling with magnet-length preservation, the Dscale amplification formula on
worked golden values, invalid-mode and zero-drift guards, and the
in-place-mutate-and-return contract. The tests use lightweight mock elements, so
they validate the scaling math without requiring a lattice file.


Validation

  • Both changes are covered by new/updated unit tests.
  • The full test suite passes with no new failures relative to main.
  • The diff touches only pySC/core/magnet.py, pySC/utils/sc_tools.py, and their
    two test modules.

thellert added 2 commits July 12, 2026 13:41
Negate the integrated order-1 B component (PolynomB[0]) in Magnet.update()
so a positive corrector setpoint produces a positive physical horizontal
kick, matching AT's convention dx' = -PolynomB[0]*L. Restrict the flip to the
order-1 term; integrated higher-order B strengths (e.g. an integrated
quadrupole B2L -> PolynomB[1]) and skew A components are left unchanged.
Mirror the flip in the state() diagnostic so it agrees with the stored value.
Add tests for order-1 (negated), order-2 (not negated), and A (not negated).
Replace the placeholder with scale_circumference(ring, circ, mode='rel'),
which scales a ring's circumference by adjusting drift lengths only, preserving
magnet lengths. Support relative (default) and absolute modes and guard against
a ring with no drift elements. Add unit tests covering both modes, magnet-length
preservation, the Dscale formula, and the error guards.

@kparasch kparasch left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For point 1, there is already a flag that can be enabled to invert the sign of the setpoints with respect to the normalized strength values. E.g. to get a B1L component that behaves like KickAngle, they can be declared like:

magnets:
  sextupoles:
    regex: ^S[DF]
    components:
      - B1L: magnet_calibration
      - A1L: magnet_calibration
    invert:
      - B1L

Without inversion, B1 follows PolynomB[0], B2 -> PolynomB[1], A1 ->PolynomA[0], of pyAT.

For point 2, the function is ok but to use it one needs to be careful. If one saves and reload the SC object, the circumference change will be lost. This is because the SC object is stored relative to the lattice.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants