Skip to content

Trace local Lorentz constant state - #163

Open
krystophny wants to merge 6 commits into
diag/flux-pumping/local-lorentz-projectorfrom
diag/flux-pumping/local-lorentz-stage-trace
Open

Trace local Lorentz constant state#163
krystophny wants to merge 6 commits into
diag/flux-pumping/local-lorentz-projectorfrom
diag/flux-pumping/local-lorentz-stage-trace

Conversation

@krystophny

@krystophny krystophny commented Jul 13, 2026

Copy link
Copy Markdown
Member

Summary

Add an opt-in stage trace that evaluates the exact pitch-independent Lorentz state at three points in every single-ripple propagator:

  1. against the assembled sparse equations before factorization;
  2. after one direct sparse solve with exact constant boundary data;
  3. in the extracted boundary map already traced by Diagnose local Lorentz projector compatibility #162.

This PR is stacked on #162. It is the minimum measurement needed to localize the finite-resolution constant-distribution transport defect before changing the solver.

The internal state uses the code's documented unknown

[
Y_k=\int_{\eta_{k-1}}^{\eta_k}f,d\eta,
]

including the moving final-band width (1/B-\eta_N) and the reversed storage order of the counter-passing branch. The sparse right-hand side applies those exact widths only at the incoming co-passing and counter-passing boundaries. The matrix residual and solved-state error are appended to NEO2_LOCAL_PROJECTION_TRACE_FILE; with that variable unset, the new path returns before allocation and does not call the extra solve.

Decision rule

  • Nonzero sparse residual: discretization or mirror matching is inconsistent with constant (f).
  • Sparse residual closes but solved state does not: sparse factorization or solve is responsible.
  • Solved state closes but Diagnose local Lorentz projector compatibility #162's boundary map does not: propagator extraction is responsible.

No correction is applied in this PR. The measured stage will determine the smallest justified fix.

Preserved invariants

  • Lorentz collision and physical source terms are unchanged.
  • Pitch and spatial discretizations, solver tolerances, and convergence criteria are unchanged.
  • Fourier convention, CGS units, periodic streaming, and mirror boundary conditions are unchanged.
  • ABI, serialized propagator layout, and generated physics outputs are unchanged.

Verification

Test fails on main

Main has no executable check for the exact internal constant state or its sparse residual:

Test project .../build
No tests were found!!!
Errors while running CTest

Test passes after fix

$ fo test lorentz_projection_diagnostic_test
lorentz_projection_diagnostic_test PASS 0.06s
Summary: 1 passed, 0 failed, 0 skipped

$ fo
Static: OK
Build: OK
Tests: OK
Lint: OK
Fmt: WARN (legacy ripple_solver_axi_test.f90)
All stages passed

Diagnosis and successor

The stage trace localized the defect to a topology crossing, not the sparse
factorization, boundary-map extraction, pitch transfer, joining, or the Lorentz
collision stencil. In propagator 192, the number of passing bands falls from 27
to 26 between spatial steps 679 and 680. The old placement routine does not
align the crossing sample when that sample is closer than both neighbours. The
constant-distribution widths on the two sides then differ by
3.365342566301611e-4; multiplication by the streaming coefficient produces
the complete 5.1395606791432513e-2 sparse row residual. Collision terms cancel
on the same row.

PR #164 supplies the stacked correction and regression. At its pinned head,
the worst normalized sparse constant residual is 1.0147e-16, the worst solved
constant-state error is 3.0437e-14, and the full 199-propagator reconstruction
completes with the same eight adaptive retries as the stacked base. A retained
stage-0 trace closes right transport at 2.2289e-14 and projector intertwining
at 2.3275e-14. The failure-free campaign remains open. This PR remains
diagnostic-only.

@slopqueue slopqueue Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review verdict: Approve

I have thoroughly reviewed the diff, the surrounding code, the test file, and the caller integration. Here is my review.


Summary: PR #163 adds "constant state" null-space diagnostic tracing to the Lorentz projection diagnostics module, with routines to assemble a reference constant solution, compute sparse-matrix residuals, and write trace files. Tests and the QL caller integration are updated to exercise the new routines.

Findings:

  1. minor lorentz_projection_diagnostics_mod.f90:370-388decode_state_index's search loop do while (step < iend) exits when step == iend, leaving step at the last step. This is the correct result, but the loop semantics are non-obvious; a brief comment noting that step is intentionally left at the last matching step would aid maintainability.
  2. minor lorentz_projection_diagnostic_mod.f90:318-368record_local_constant_row accesses bhat(column_step) after decoding. If decode_state_index returns an out-of-range step (e.g., for an invalid index that passes the ierr=0 check but yields step outside bhat bounds), this would be an out-of-bounds access. The current validation guards against index < 0 and ibeg > iend, but not against step being outside bhat's declared bounds. Consider adding a bounds check on step before using it to index bhat.

Verdict: Approve — The change is well-structured with comprehensive input validation, thorough tests covering all new public routines, and properly guarded integration into ripple_solver_axi_test.f90 behind the isw_lorentz.EQ.1 guard. The minor items are suggestions for robustness, not blockers.

@slopqueue slopqueue Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review verdict: Comment

Summary

PR #163 adds optional diagnostic tracing for the Lorentz constant-state projection in the QL ripple solver, exposed through a new lorentz_projection_diagnostics_mod API and gated by the NEO2_LOCAL_PROJECTION_TRACE_FILE environment variable. The diff is additive (331 lines across three files), with caller integration in ripple_solver_axi_test.f90 and test coverage in TEST/test_lorentz_projection_diagnostic.f90.

Findings

  1. major NEO-2-QL/ripple_solver_axi_test.f90 (inline decode block) — The caller duplicates decode_state_index logic with an inline DO WHILE(constant_sparse_index...) block instead of calling the already-public decode_state_index subroutine from the diagnostics module. This risks divergence between the two implementations and defeats the purpose of extracting decode_state_index into a reusable routine. Required fix: replace the inline decode block with a call to decode_state_index, or document why the inline copy is necessary.

  2. minor NEO-2-QL/lorentz_projection_diagnostics_mod.f90 (initialize_output) — initialize_output opens a file with open(newunit=iunit, ...) but the unit is local; subsequent record_* routines call initialize_output again and open independently. If the file is opened multiple times or the unit is lost, traces may be interleaved or truncated. Consider storing the unit in a module-saved variable and guarding with row_output_initialized/output_initialized consistently.

  3. minor TEST/test_lorentz_projection_diagnostic.f90 — The test exercises assemble_local_constant_state and compute_sparse_constant_residual with small fixed arrays, but does not verify the decode_state_index decoding against known (step, laguerre, sigma, band) tuples. Since the caller now relies on this decoding (see finding 1), a direct unit test of decode_state_index would close the gap.

Verdict

Comment — The diagnostic functionality is reasonable and opt-in, but the inline decode duplication in the caller should be resolved (or justified) before merge to avoid maintenance divergence.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant