Fix H-corrector sign convention and implement scale_circumference#43
Open
thellert wants to merge 2 commits into
Open
Fix H-corrector sign convention and implement scale_circumference#43thellert wants to merge 2 commits into
thellert wants to merge 2 commits into
Conversation
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
reviewed
Jul 21, 2026
kparasch
left a comment
Owner
There was a problem hiding this comment.
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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
1. Correct the sign of the integrated dipole/corrector term in
Magnet.update()Magnet.update()divides an integrated-strength link value by the magnet lengthand adds it to
PolynomB/PolynomA. For the order-1 normal term(
PolynomB[0], the dipole/corrector field) this did not account for AT's kicksign convention.
In AT, the horizontal kick from
PolynomB[0]isΔx' = -PolynomB[0]·L. Apositive
PolynomB[0]therefore produces a negative horizontal kick, so apositive 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:
Scope notes:
order == 1(PolynomB[0]). Integratedhigher-order B strengths — e.g. an integrated quadrupole
B2L→PolynomB[1]used by the shifted-quadrupole feed-down path — must not beflipped, as that would invert focusing. Tests cover this explicitly.
A) components are never flipped; the convention flip is specific to thenormal dipole term.
state()diagnostic mirrors the same flip so its per-link printout agreeswith the stored
PolynomB[0].Tests (
tests/core/test_magnet.py):B[0] == -3.0for the worked example);B[1] == 3.0);A[0] == 3.0).2. Implement
scale_circumferenceinutils/sc_tools.pyscale_circumferencewas present only as a commented-out placeholder. This PRprovides a working implementation that scales a ring's circumference by adjusting
drift lengths only, leaving magnet lengths untouched:
mode='rel'(default):circis a multiplicative factor (e.g.1 + 1e-6);the new total circumference is
circ · C.mode='abs':circis the desired absolute circumference in metres.Dscale), somagnet 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 arelative fractional stretch is the common circumference-error use (a small
multiplicative factor around 1).
'abs'remains available for setting an exacttarget length.
Tests (
tests/utils/test_sc_tools.py): relative/absolute modes, drift-onlyscaling with magnet-length preservation, the
Dscaleamplification formula onworked 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
main.pySC/core/magnet.py,pySC/utils/sc_tools.py, and theirtwo test modules.