Repository navigation
Add a multi-n perturbed-equilibrium regression case (+ threaded verification) for singular-coupling threading #334
Description
Motivation
PR #325 parallelized the singular-coupling loop in PerturbedEquilibrium/SingularCoupling.jl over resonant (surface, n) pairs (and the NTV / field-reconstruction paths). During review we found that the regression harness has no multi-n perturbed-equilibrium case, and CI never sets JULIA_NUM_THREADS, so:
diiid_n1is single-n (nn = 1).solovev_multi_nis multi-n but ideal-only — it does not run perturbed equilibrium / singular coupling.- Every
@threadsregion runs on 1 thread in CI, so none of the "bit-identical across thread counts" behavior is exercised by automation.
The specific untested path: for a surface with (near-)integer q, the same surface index s appears on multiple rows (n=1 → m, n=2 → 2m, …). This repeated-s path is only reachable in a multi-n PE run. A write-only data race on sing[s].grri/.grre lived exactly here (removed in #325 424463c); any future per-(s,n) singular-surface state would reintroduce this hazard with no regression coverage to catch it.
Ask
Add a multi-n perturbed-equilibrium regression case whose equilibrium + n-range place an integer-q rational surface in range at more than one n, and verify the threaded coupling output is deterministic vs serial and across thread counts.
Subtleties to work through (why this warrants its own PR)
- Case selection. Need an equilibrium where an integer-
qsurface resonates at multiplenwithin both[nlow, nhigh]and[mlow, mhigh]— i.e.n·qlands on an in-range integermfor ≥2 values ofn. This must be deliberately constructed, not assumed. - New example deck. No existing example runs multi-n PE. Likely need a new
examples/…_multi_n_pe_example/gpec.toml(Solovev-based to keep runtime low) with[PerturbedEquilibrium]enabled and[ForcingTerms]forcing spanning the n-range. Follow the TOML annotation conventions inCLAUDE.md. - Quantity extraction. The coupling outputs (
C_resonant_*,resonant_*, island half-widths,delta_prime,b^r) are per-row(surface, n). The harness case needs to track them as arrays with a stable, n-keyed ordering so diffs are meaningful. - Thread-count determinism. The harness currently caches by commit, not by thread count. Decide how to assert bit-identity between serial and
JULIA_NUM_THREADS>1(e.g. a dedicated threads-vs-serial test outside the commit-cached harness, or a harness extension) and how BLAS pinning interacts. - CI. Consider a threaded CI job (or a targeted threads-vs-serial unit test) so the
@threadspaths inSingularCoupling,FieldReconstruction, andKineticForces/Compute.jlare actually exercised — today they are not. - Runtime budget. Multi-n PE is heavier; keep the case small enough for CI/harness time.
Context
- Review + fixes: PerturbedEquilibrium & KineticForces: parallelize serial workflow loops (coupling, NTV torque) + reduce allocations #325 (comment: PerturbedEquilibrium & KineticForces: parallelize serial workflow loops (coupling, NTV torque) + reduce allocations #325 (comment))
- Fix commit removing the race and guarding BLAS restore: 424463c
Activity
jhalpern30 commentedon Jul 29, 2026
@logan-nc assuming I am understanding this right, I think this is more than just adding a regression test - the multi-n code needs to be fully propagated into the perturbed equilibrium module. I think right now, there are some assumptions that n = nlow throughout the PE code.
I think this will ultimately need to be a rather large PR from me as a step towards fully 3D PE, and in that PR I will add unit tests + a regression case for multi-n PE.
Agree?
jhalpern30 commentedon Jul 29, 2026
In one of my WIP branches, I think I actually worked out the current flags we need to block PE calculations - I think we currently only do the first one:
jhalpern30 commentedon Sep 29, 2026
Closed by #477. PerturbedEquilibrium (PE) now runs multi-n, and the new
diiid_multi_nregression case covers the repeated-surface path this issue asked for:SingularCoupling.Checking that threaded results match serial, and running CI with more than one thread, are not covered. They move to #479, along with the two missing PE guards.