Preserve geometry at pitch crossings

[krystophny]

Problem

PR 164 makes the discrete constant-distribution null exact, but its crossing
repair changes only bhat_mfl at a grid point. The coordinate, metric, drift,
and derivative arrays remain sampled at the old point. This creates an
internally inconsistent geometry that moves with spatial resolution.

Change

• Locate each simple trapped-passing boundary from the bracketed root
  1 - eta B(phi) = 0 in the existing six-point Lagrange representation.
• Move the nearer interior node to the root without changing the node count,
  pitch bands, array bounds, ABI, or serialized propagator layout.
• Interpolate B, h_phi, dlogB/ds, bcovar_s_hat, and geodcu from one
  unchanged stencil. Recompute dlogB/dphi and d(bcovar_s_hat)/dphi from
  the same polynomial.
• Apply the same path to the Arnoldi and axisymmetric QL solvers. Derived NTV
  arrays and nonuniform cell factors are built after alignment.
• Request spatial refinement when the bracket is lost, the root is not simple,
  one cell crosses more than one pitch band, the field is not monotone away
  from its minimum, or two crossings require the same node.

The magnetic-field representation, CGS units, pitch measure, mirror and
periodic endpoint conventions, node order, and solver storage layout are
unchanged. This PR changes where existing field data are sampled; it does not
change the collision operator or introduce a new source.

This PR is stacked on #164 and should remain unmerged until the pinned spatial
and pitch campaigns pass.

Verification

Test fails on the base branch

With the new behavioral test applied before the implementation:

$ fo test phi_crossing_alignment_test
TEST/test_phi_crossing_alignment.f90: Function 'claim_crossing_target' has no IMPLICIT type
Summary: build failed

Test passes after fix

$ fo test phi_crossing_alignment_test
phi_crossing_alignment_test                PASS       0.06s
Summary: 1 passed, 0 failed, 0 skipped (  0.06s)

$ fo
Static: OK (356 modules, 356 changed, 356 affected)
Build: OK
Tests: OK
Lint: OK
Fmt: WARN
All stages passed (.7s)

The format warning is limited to the two pre-existing legacy solver files;
the module and behavioral test are formatted.

[krystophny]

Spatial campaign 18573023 completed at 1,920/3,840/7,680 steps on the pinned PR head. All three component runs exited zero with no adaptive recoveries; topology stayed fixed and the maximum qflux relative error was 3.48e-14. The two harmonic-response components were (-13.6163181,-70.1506689), (-13.6152840,-70.1478445), and (-13.6134972,-70.1396413). Successive relative changes were (7.59e-5,4.03e-5) and (1.31e-4,1.17e-4): both are below 1e-3, but the final change grows rather than contracts, so this does not pass the spatial convergence gate.

The submission also omitted the local-projection and join-end trace flags. It therefore did not record the constant-distribution closure or global Lorentz correction at each resolution and is retained only as a rejected diagnostic. Campaign 18573042 repeats 3,840/7,680 and extends to 15,360 steps with both traces enabled. The incomplete pitch campaign was cancelled and replaced by diagnostic-complete campaign 18573043 at 30/60/120/240 bands. Collision physics, source, coordinates, units, boundary conditions, ABI, and serialized layout remain unchanged. No pointwise current or physical profile is accepted pending both corrected gates.

[krystophny]

Bounds/gate defect fixed in 6aec6fc

Defect

rearrange_phideps passed ubound(phi_divide, 1) to
align_phi_crossing_geometry as the physical scan bound. The dummy
phi_divide was declared dimension(1:iend) with
iend = ub_mag + 2*modify_br, while both callers allocate
phi_divide(1:ub_mag). With right boundary-layer modification active
(modify_br = 1, reachable through ripple_solver_ArnoldiO2; the
axisymmetric solver pins modify_br = 0 via its long-standing
goto 10), three things went wrong:

1. The crossing scan ran over 0:ub_mag+2 and ingested the two
   duplicated padding nodes, so bracketed_phi_crossing's
   strict-monotonicity guard (phi(iend-1) == phi(iend)) rejected
   every crossing regardless of location - each propagator with a
   pitch crossing requested refinement forever.
2. A padding-region rejection clamped failed_interval with
   min(ub_mag+2, ...) and wrote the refinement flag past the actual
   phi_divide(1:ub_mag) extent.
3. The caller's maxval(phi_divide) > 1 gate never saw that flag, so
   the ierr = 3 retry was silently lost and the solver continued
   with uninitialized delt_pos/delt_neg/fact_*.

The pinned HELCORE campaigns (18573042/18573043) ran the axisymmetric
solver with modify_br = 0 (iend == ub_mag), so their recorded
results are unaffected.

Fix

• Pass the physical ub_mag into rearrange_phideps explicitly and
  forward it to align_phi_crossing_geometry; the crossing scan now
  covers 0:ub_mag on both sides of the field minimum.
• Size the phi_divide dummy as (1:ub_mag), matching every actual
  argument, so the blanket phi_divide = 1 and the rejection flag stay
  in bounds and the caller gate fires for every rejection.
• Clamp the legacy sub-interval split writes to
  max(1, ib+1):min(ie, ub_mag); this also closes the pre-existing
  below-bounds write for modify_bl = 1 (ib + 1 = ibeg + 1 = -1).

With modify_br = 0 the passed bound equals iend, all clamps are
identities, and the arithmetic is unchanged - the pinned campaign
behavior is bit-identical.

Verification

New behavioral test TEST/test_crossing_geometry_padding.f90
(crossing_geometry_padding_test): a 41-node parabolic field with one
pitch band and two simple crossings, run unpadded and with
modify_br = 1 padding (boundary value above all physical values, and
below the crossing band so the padding mimics an extra crossing), plus
a padded case with a non-monotone physical field. It requires bitwise
agreement of the padded and unpadded aligned geometry, no spurious
refinement, untouched padding nodes, consistent integration steps, and
the rejection flag inside phi_divide(1:ub_mag).

Test fails on this PR's previous head 0345e33 (call adapted to the old
signature, implicit interface exactly as the solver callers use):

$ fo test crossing_geometry_padding_test
crossing_geometry_padding_test             FAIL       0.00s
ERROR STOP FAIL: benign padding triggered spurious refinement

Test passes after the fix, along with the existing crossing tests:

$ fo test crossing_geometry_padding_test phi_crossing_alignment_test phi_divider_parity_test phi_placement_diagnostic_test
phi_crossing_alignment_test                PASS       0.06s
phi_placement_diagnostic_test              PASS       0.06s
phi_divider_parity_test                    PASS       0.07s
crossing_geometry_padding_test             PASS       0.06s
Summary: 4 passed, 0 failed, 0 skipped (  0.25s)

$ fo
Build: OK
Tests: OK
Lint: OK
All stages passed (51.1s)

ctest: 43/44 pass; the single failure is the pre-existing
external-data case Extended_QL_Multispecies_Read (needs
NEO2_TEST_PATH data not present locally), excluded from the local
pipeline by the external-data classification.

NEO-2-PAR keeps its own separate rearrange_phideps (old signature,
same latent extent mismatch); porting the alignment there or
documenting the divergence stays on the consolidation list.

[slopqueue]

Review verdict: Approve

Review of PR #165: Crossing Geometry Alignment and phi_divide Bounds Fix

Summary: This PR fixes an out-of-bounds bug in phi_divide (declared dimension(1:iend) but allocated 1:ub_mag), replacing the old nearest-neighbor crossing alignment with 6-point Lagrange interpolation and bisection root-finding. The core fix changes phi_divide's declaration to dimension(1:ub_mag) matching its allocation, and adds max(1, ib+1) / min(ie, ub_mag) clamping on all writes. Two new test files exercise the alignment and padding regression paths. The changes are well-structured and the new module phi_crossing_alignment_mod cleanly separates the interpolation logic.

Findings:

1. minor NEO-2-QL/ripple_solver_ArnoldiOrder2_test.f90:~5410 — Debug file I/O (open(111, file='phi_placement_problem.dat'...) remains in align_phi_crossing_geometry, writing to disk on every crossing. This is pre-existing (carried from old fix_phiplacement_problem`), not a regression, but it should eventually be guarded behind a debug flag or removed for production builds.

2. minor NEO-2-QL/ripple_solver_axi_test.f90:~3409 — rearrange_phideps_old is still present in the file but no longer called (dead code). Consider removing in a follow-up cleanup.

3. minor NEO-2-QL/ripple_solver_axi_test.f90:~773 — unused_dbcovar_mfl is allocated, zeroed, passed to rearrange_phideps as dbcovar_s_hat_dphi_mfl, and deallocated without being used. This is harmless but suggests the bcovar_column path is intended for future use; a comment would help.

4. minor NEO-2-QL/phi_crossing_alignment_mod.f90:~65 — stencil_left computation uses crossing_stencil_size/2 (integer division, 6/2=3), which is correct for Fortran integer division. The +1 ensures the bracket node is centered in the stencil. No issue, noting for clarity.

Verdict: Approve — The core OOB fix is correct (declaration now matches allocation, all writes are bounds-clamped). The new alignment method is sound, well-tested, and the claim_crossing_target double-assignment guard is a good defensive measure. The nder change from 0→1 correctly enables derivative computation. No blockers or major issues found. The minor items are pre-existing or cosmetic.

[slopqueue]

Review verdict: Request changes

Review of PR #165: Phi crossing geometry alignment refactor

Summary: This PR refactors phi-crossing geometry handling in the QL test variants. A new phi_crossing_alignment_mod introduces bracketed root-finding (bracketed_phi_crossing), crossing-target claiming (claim_crossing_target), and status codes. rearrange_phideps gains a ub_mag parameter so phi_divide is allocated to the physical extent (1:ub_mag), with writes clamped via max(1,ib+1):min(ie,ub_mag) to protect padding nodes. align_phi_crossing_geometry replaces fix_phiplacement_problem, scanning only the physical grid for crossings and rejecting non-simple roots. Two new test programs are added.

Findings:

1. [major] TEST/test_crossing_geometry_padding.f90:122 — Unbalanced parentheses in allocate statements. Line 122 has allocate (dbcovar(0:iend), delt_pos(0:iend), delt_neg(0:iend)) with an extra ), which is a compile error. This newly added test file cannot build as written. Fix: Remove the extra closing paren on each affected allocate statement.

2. [major] TEST/test_phi_crossing_alignment.f90:54-56 — Incorrect parenthesization in test assertions makes them vacuously true. Line 54 computes abs(sum(weights*bhat_profile(...))) * eta_root - 1.0 instead of abs(sum(...)*eta_root - 1.0). The abs() closes before *eta_root, so the test checks |sum| * eta - 1 rather than |sum*eta - 1|. It passes even with wrong weights. Fix: Move the closing paren of abs() to encompass *eta_root - 1.0_real64.

3. [minor] NEO-2-QL/ripple_solver_ArnoldiOrder2_test.f90:~5370 — align_phi_crossing_geometry opens and writes to 'phi_placement_problem.dat' on every crossing during integration. This debug file I/O in the hot path is inherited from the old fix_phiplacement_problem and should be removed or guarded by a debug flag.

4. [minor] NEO-2-QL/phi_crossing_alignment_mod.f90:~100 — The strict monotonicity check any(phi(ibeg+1:iend) <= phi(ibeg:iend-1)) has no tolerance. Tiny floating-point noise in real MHD equilibria could cause spurious crossing_non_simple rejections. Consider a relative tolerance on the phi differences.

Verdict: Request changes — The test files contain syntax errors (unbalanced parentheses) and logic errors (vacuous assertions) that would cause compile failures or silently pass with incorrect values. These must be fixed before the PR can be merged.