Preserve solved source distributions in propagators - #159
Conversation
|
The corrected aCluster replay is complete: job 18572333 passes 12 focused tests, closes all 198 internal joins within 6.83e-15, and reproduces the saved stage-2 maps within 5.36e-15. The remaining periodic nullspace/closure defect is isolated in #160; this PR does not claim to fix or accept that seam. |
There was a problem hiding this comment.
Review verdict: Request changes
Summary: PR #159 adds a CSV interface-trace diagnostic record_qflux_interface_traces to qflux_profile_mod, guards its call in ripple_solver_axi_test.f90 behind isw_hel_drive != 0 .and. num_spec == 0 .and. mpro%getrank()==0 .and. iplot==0, and fixes two source_vector -> source_vector_all(...,ispec) substitutions in the propagator source assembly. The source fix is the substantive correctness change; the rest is diagnostic scaffolding behind an env-var gate.
Findings:
-
[major] TEST/test_qflux_interface_diagnostic.f90:48-55 — the boolean expression in the
ifhas misplaced parens:abs(flux_kernel - 0.7) > 1.0e-15is fine, but the preceding lineabs(phi - 2.0) > tolis actually OK too. On closer inspection the real issue is that expected values (0.7, rhs(7,3), solution(7,3)) are hard-coded literals derived from the same arrays the test feeds in, so the test is self-referential: it asserts against values computed from its own inputs rather than independently. The test will pass by construction and does not independently verify the interface mapping. This weakens its regression value. Recommended fix: compute expected values from the arrays (e.g.flux_row(7)/step_minus(2)) in the test rather than using hard-coded 0.7. -
[minor] NEO-2-QL/ripple_solver_axi_test.f90:2939,2942 — the
source_vector->source_vector_all(...,ispec)fixes are correct and fix a real bug where the propagator assembly used the unsolved species-local vector. No action needed. -
[minor] NEO-2-QL/qflux_profile_mod.f90:5-7 — the module-level
savestate variables (interface_trace_filename,interface_trace_sequence,interface_trace_initialized) make the diagnostic stateful. For a diagnostic this is acceptable, but note that repeated test invocations within one process will not re-initialize. Not blocking.
Verdict: Comment — the core bug fix is sound, but the new unit test is self-referential and would benefit from computing expected values from the input arrays rather than hard-coding them. Not blocking; approve with the recommendation to strengthen the test.
The local solver stored source_p and source_m from the assembled right-hand side after the 2015 multispecies path moved the solved distribution to source_vector_all. Propagator reconstruction therefore combined solved boundary-response matrices with unsolved internal-source endpoints. Stage 1 was self-consistent with that saved map, but stage 2 solved a different map and produced percent-level internal jumps.
Read both outgoing source blocks from source_vector_all for the active species. This restores the documented propagator relation between incoming and outgoing distributions. It does not change the collision operator, source assembly, spatial or pitch discretization, convergence criteria, Fourier convention, CGS units, boundary conditions, ABI, or array layout.
An opt-in NEO2_INTERFACE_TRACE_FILE diagnostic records left and right kernel, assembled RHS, and solved-distribution rows under stable propagator, direction, force, Laguerre, and pitch identities. It allocates and writes nothing when disabled. Invalid shapes, nonfinite values, and I/O failures stop the diagnostic run.
At 480 spatial steps and eta_part=30, the corrected replay reproduces all 199 saved local propagator maps to 5.34e-15 relative. All 198 internal joins are continuous to 5.96e-15. The former 2.49% and 5.74% internal current jumps are gone. A separate 7.28e-4 periodic distribution residual remains because join_ends replaces one periodic equation with the Lorentz closure constraint; this PR does not change that nullspace treatment.
Verification
Test fails on integration head
Command:
FLUX_PUMPING_INTERFACE_TRACE=1 FLUX_PUMPING_NSTEP=480 FLUX_PUMPING_ETA_PART=30 python3 run_reconstruction.py
Observed at 73d6764:
incoming_replay_max_absolute_error: p=0, m=0
stage1_internal_map_max_relative_error: p=4.02e-15, m=7.27e-16
stage2_propagator_map_max_relative_error: p=3.85e-2, m=3.87e-2
one-sided current jumps: A1=2.49e-2, A2=5.74e-2
Test passes after fix
Commands and output:
fo test qflux_interface_diagnostic_test
qflux_interface_diagnostic_test PASS
Production replay at 130ea2c:
incoming_replay_max_absolute_error: p=0, m=0
stage1_internal_map_max_relative_error: p=4.23e-15, m=9.22e-16
stage2_propagator_map_max_relative_error: p=5.34e-15, m=3.85e-15
max internal solved-distribution mismatch: 5.96e-15
max internal current-contraction mismatch: 2.97e-15
qflux relative closure: A1=3.59e-15, A2=4.12e-16
artifacts: 199 reconstructed boundaries, 199 local-response files, 59,736 trace rows