Add native dead and wrong-polarity BPM error model#44
Conversation
Model dead BPMs and per-plane polarity reversal as config-driven BPM errors. BPMSystem gains optional dead, polarity_x and polarity_y arrays; each capture path multiplies the signal by the polarity before noise and overwrites dead BPMs with amplified noise, guarded by an alive mask so a dead BPM downstream of beam loss stays NaN instead of fabricating a reading. configure_bpms applies fraction_dead and fraction_wrong_polarity per BPM category and registers dead BPMs in bad_bpms. Add RNG.choice for reproducible subset selection. Include regression tests for the guard and the configuration invariants.
kparasch
left a comment
kparasch
left a comment
There was a problem hiding this comment.
I think this is a very nice model to include. I have only requested some minor changes I would like
| n_dead = max(1, round(len(cat_indices) * fraction_dead)) | ||
| dead_idx = SC.rng.choice(len(cat_indices), size=n_dead, replace=False) | ||
| SC.bpm_system.dead[cat_indices[dead_idx]] = True | ||
| SC.tuning.bad_bpms.extend(cat_indices[dead_idx].tolist()) |
There was a problem hiding this comment.
dead bpms should not be passed to SC.tuning.bad_bpms . The user should declare the bad bpms themselves through appropriate measurements
|
|
||
| dead: Optional[NPARRAY] = None | ||
| polarity_x: Optional[NPARRAY] = None | ||
| polarity_y: Optional[NPARRAY] = None |
There was a problem hiding this comment.
These can be:
dead: NPARRAY] = np.array([])
polarity_x: NPARRAY] = np.array([])
polarity_y: NPARRAY] = np.array([])
| for field in BPM_FIELDS_TO_INITIALISE_ONES: | ||
| if not len(getattr(self, field)): # array is empty | ||
| value = getattr(self, field) | ||
| if value is None or not len(value): # optional/unset or empty array |
There was a problem hiding this comment.
no need for this if dead, polarity_x, polarity_y is declared as np.array([]) by default
| fake_orbit_x = (rotated_orbit[0] - self.offsets_x) * (1 + self.calibration_errors_x) + noise_x | ||
| fake_orbit_y = (rotated_orbit[1] - self.offsets_y) * (1 + self.calibration_errors_y) + noise_y | ||
| pol_x = self.polarity_x if self.polarity_x is not None else 1.0 | ||
| pol_y = self.polarity_y if self.polarity_y is not None else 1.0 |
There was a problem hiding this comment.
no need for the if-clause, polarity_x, polarity_y will always be initialized.
|
|
||
| fake_trajectory_x_tbt = np.zeros([len(self.indices), n_turns]) | ||
| fake_trajectory_y_tbt = np.zeros([len(self.indices), n_turns]) | ||
| pol_x = self.polarity_x if self.polarity_x is not None else 1.0 |
There was a problem hiding this comment.
same: no need for the if-clause, polarity_x, polarity_y will always be initialized.
|
|
||
| fake_trajectory_x_tbt = np.zeros([len(self.indices), n_turns]) | ||
| fake_trajectory_y_tbt = np.zeros([len(self.indices), n_turns]) | ||
| pol_x = self.polarity_x if self.polarity_x is not None else 1.0 |
There was a problem hiding this comment.
same: no need for the if-clause, polarity_x, polarity_y will always be initialized.
| if self.dead is not None and self.dead.any(): | ||
| dead_alive_x = self.dead & ~np.isnan(fake_x) | ||
| dead_alive_y = self.dead & ~np.isnan(fake_y) | ||
| fake_x[dead_alive_x] = noise_x[dead_alive_x] * 10 |
There was a problem hiding this comment.
I don't like the hard-coded "10" factor here. Maybe it can be declared as one of the fields in the BPMSystem class:
class BPMSystem(BaseModel, extra='forbid'):
...
dead_bpm_noise_factor: float = 10
...
2 participants
Motivation
Realistic commissioning simulations need to model imperfect BPMs:
position.
Today these can only be emulated by post-processing BPM output outside the
library. This PR makes both first-class, YAML-configurable BPM errors, applied
consistently through every reading path.
What the feature does
BPMSystem(pySC/core/bpm_system.py)dead(bool),polarity_x,polarity_y(float, default all-ones). They default to
Noneand are populated duringconfiguration; existing construction is unaffected.
polarity_x/polarity_yare added to the ones-initialised BPM field set, andinitialize_empty_arrays()fills them consistently with the other per-BPMarrays.
capture_orbit,capture_injection,capture_kick):is added (a
Nonepolarity is treated as unity, so unconfigured systems areunchanged);
guarded by an alive mask (see below).
Dead-BPM alive-mask guard
A dead BPM should only fabricate a noise reading where the beam actually reached
it. Downstream of a beam-loss point the reading is already
NaN(the beam neverarrived), and overwriting it with
noise*10invents phantom readings thatinflate transmission/reach metrics. The override is therefore masked:
Dead BPMs upstream of the loss still emit amplified noise; dead BPMs downstream
stay
NaN.Configuration (
pySC/configuration/bpm_system_conf.py)configure_bpmsreads two optional per-BPM-category fractions:fraction_dead→ marksmax(1, round(n·fraction))BPMs in the category deadand registers them in
SC.tuning.bad_bpms;fraction_wrong_polarity→ flipsmax(1, round(n·fraction))BPMs to-1.0polarity, drawn independently for x and y.
RNG (
pySC/core/rng.py)Adds
RNG.choice(a, size=None, replace=True), a thin seeded wrapper over theunderlying generator's
choice, used for the reproducible random subsetselection above. This keeps all randomness routed through the seeded
RNGsoruns remain reproducible.
Scope / compatibility
deadunset,polarities unity). No existing public behavior changes.
initialize_empty_arrays()is preserved and extended, not removed.existing optional-field handling.
Validation
NaNguard on thetracking path (
capture_injection) and as acapture_orbitguard unit test;and the configuration invariants (dead count,
bad_bpmsmembership,-1.0polarity), written to be robust to RNG draw order.
main.pySC/core/bpm_system.py,pySC/configuration/bpm_system_conf.py,pySC/core/rng.py, and the BPM testmodules.