Stratified ("quiet") resampling - #149
Conversation
…ow stratified sampling at poor ratios
|
Fable 5 review: Review: PR #149 — Stratified ("quiet") resampling Overview Adds stratified_resample_particles() to statistics.py, exposes it via a new ParticleGroup.stratified_resample() method and a method= kwarg on resample(), plus a demo notebook wired into mkdocs and three new tests. I verified the core math (strata edges via linspace(0, m, n+1).astype(int) always yield width ≥ 1 when n ≤ m; offsets stay in-stratum; charge is preserved; id is carried through when present), and the new tests pass in beamphysics-dev (3 passed, 1.16s). The implementation is clean, well-documented, and backward compatible. No correctness bugs found — the items below are design and polish. Main design point: the weight check happens after the ratio fallback In stratified_resample_particles (beamphysics/statistics.py:592-601), the fallback to random runs before the variable-weight rejection. So whether variable weights are accepted depends on n:
Asking for more output particles succeeds while asking for fewer errors, which is backwards from what a user would guess, and there's no signal that the fallback occurred. Since Ken's feedback removed the warnings, the silent fallback is presumably intentional for the ratio itself — but I'd still move the weight check above the fallback so the "stratified requires constant weights" contract holds unconditionally. If the current behavior is deliberate, it should at least be stated in the docstring. Other suggestions
On the tests ("not sure how useful they are") They're genuinely useful — good coverage of the happy path (count, status, charge preservation, equal weights, picks-from-source), dispatch through resample(method=...), all three error branches, and the ratio fallback. Two gaps:
Minor: set(np.asarray(Q.t)).issubset(...) works (exact copies, so float equality is safe) but np.isin(Q.t, alive.t).all() is the idiomatic form. Verdict Solid PR — mergeable as-is from a correctness standpoint. The one thing I'd change before merge is the weight-check ordering (or an explicit docstring note if the current behavior is intended); the RNG seedability and a stratification-property test are worthwhile follow-ups. |
|
@ChristopherMayes All suggestions implemented |
Adds the option for stratified ("quiet") resampling (in addition to the current, fully random downsampling) to
resample(). Includes a new demo notebook contrasting random vs stratified downsampling and their effects on the bunching factor of a beamThis higher noise power from random downsampling can cause collective effects like CSR to be incorrectly represented in simulation, leading to major, macroscopic divergences from high fidelity simulations; stratified downsampling can mitigate this.
Stratified downsampling can cause distortions if the new population is not much smaller than the initial population. By default, if the ratio is not more than 5,
stratified_resample_particles()will fall back to random. The user may override this behavior by settingallow_bad_sampling_ratioto True.Claude made some tests for the new function; not sure how useful they are.