Skip to content

Trace the flux-side closure correction and gate the Lorentz correction - #168

Open
krystophny wants to merge 2 commits into
diag/flux-pumping/per-level-retentionfrom
diag/flux-pumping/closure-channel-trace
Open

Trace the flux-side closure correction and gate the Lorentz correction#168
krystophny wants to merge 2 commits into
diag/flux-pumping/per-level-retentionfrom
diag/flux-pumping/closure-channel-trace

Conversation

@krystophny

Copy link
Copy Markdown
Member

Stacked on #167 (do not merge before it; owner review).

The periodic-join Lorentz solvability correction acts through two channels: the per-band source-row subtractions (traced since #167) and a per-force scalar subtracted uniformly from both flux extraction rows (untraced until now).

What this adds

  1. Flux-channel trace rows in the join-end trace: flux_factor, flux_before, flux_after, flux_scale per force, plus per-band flux_correction_m/flux_correction_p rows in the same tag,side,force,band format as the source side. Bookkeeping note now recorded in the module doc: join_ripples composes flux rows only into qflux and other flux rows, so this channel can shift the extracted transport coefficients but structurally cannot affect the pointwise reconstruction (relevant to the converged periodic-interface jump measured on the pinned Retain per-level diagnostics for closure attribution #167 campaigns: the flux channel is excluded a priori for the jump, and the trace makes its gamma-level effect auditable).

  2. NEO2_DISABLE_JOIN_LORENTZ_CORRECTION=1 as a controlled A/B switch: the periodic solve keeps the uncorrected sources and extraction rows while the trace still records the would-be correction factors; a loud audit line marks any disabled run (runners must require it, mirroring the stage-2 attribution marker). This is the decisive experiment for the interface jump after the chain-end source-row channel was excluded by the corrected attribution pass B (flux_pumping run record 19526225): if the jump vanishes with the correction disabled, it is the correction's distributed footprint on the periodic solution and not attributable to any single-propagator edit.

Tests: test_join_failure_diagnostic extended for the new recorder signature, the flux rows (uniform per-force scalar), a non-finite flux-scale rejection, and the default-off state of the switch; passes. Full local ctest: 30/30 environment-independent tests pass; the 16 external-data regression tests fail for a missing NEO2_TEST_PATH on this machine (pre-existing, unrelated).

No behavior change without the env switch: the correction arithmetic is untouched and the new trace rows appear only when NEO2_JOIN_END_TRACE_FILE is set.

The periodic-join Lorentz solvability correction acts through two
channels: per-band source-row subtractions (already traced) and a
per-force scalar subtracted uniformly from both flux extraction rows
(untraced until now).  Record the flux channel in the join-end trace
(flux_factor/flux_before/flux_after/flux_scale rows plus per-band
flux_correction_m/flux_correction_p rows in the same format as the
source side).  Since join_ripples composes flux rows only into qflux
and other flux rows, this channel can shift extracted transport
coefficients but never the pointwise reconstruction; the trace makes
that bookkeeping auditable.

Add NEO2_DISABLE_JOIN_LORENTZ_CORRECTION=1 as a controlled A/B switch:
the periodic solve then keeps the uncorrected sources and extraction
rows while the trace still records the would-be correction factors,
and a loud audit line marks the disabled run.  This is the decisive
experiment for the converged periodic-interface jump: if the jump
vanishes without the correction, it is the correction's distributed
footprint on the periodic solution and not attributable to any
single-propagator edit (the chain-end source experiment already
excluded that channel).

Extend test_join_failure_diagnostic for the new recorder signature,
the flux rows, a non-finite flux-scale rejection, and the default-off
state of the switch.

@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

Summary

PR #168 adds three diagnostic capabilities to NEO-2-QL's join_ends and qflux_profile modules: (1) a source_solution_orthogonal column in the qflux interface trace that projects out the Lorentz right-null constant coordinate from lag=0 passing orbits, (2) flux tracing (flux_factor, flux_before, flux_after, flux_scale) in join_ends to quantify how the Lorentz solvability correction shifts flux extraction, and (9 a controlled A/B switch (NEO2_DISABLE_JOIN_LORENTZ_CORRECTION) that disables the source/flux correction while still recording the would-be correction, enabling comparison runs.

Findings

  1. minor NEO-2-QL/join_ends.f90:111 — The pre-existing debug PRINT *,'nl,nr = ',nl,nr is not introduced by this PR (it's in the base), but it remains as noise in production output. Consider removing in a follow-up.

  2. minor NEO-2-QL/qflux_profile_mod.f90:186-191source_solution_orthogonal is set to source_solution(column, force) for all m, then conditionally adjusted only when m == 0. For m > 0 the "orthogonal" value is just the raw solution. This is intentional (higher Laguerre orders are already orthogonal to the constant mode by construction), and the comment explains it, but the column name could be slightly misleading in trace output for m > 0 rows. Not blocking.

  3. minor NEO-2-QL/join_diagnostics_mod.f90:43 — The .and. non-short-circuit comment on correction_switch_disabled = status == 0 .and. length == 1 .and. flag == '1' is correct defensive Fortran, but could confuse readers unfamiliar with Fortran's evaluation rules. The pre-initialization flag = '0' properly guards against undefined reads. Good practice.

Verdict: Approve

The changes are well-structured, correctly implemented, and thoroughly tested. The constant_coordinate computation (mean of co+counter-passing solution values) is mathematically sound, the flux_before/flux_after pair correctly captures the correction's effect, and the A/B switch is safely guarded with a loud stdout marker and a test verifying it's off by default. Tests are updated to cover all new parameters including non-finite rejection paths.

@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

Review of PR #168: Trace flux-side closure correction and gate the Lorentz correction

Summary: This PR extends the periodic-join diagnostic to record the previously-untraced flux-channel correction (flux_factor/flux_before/flux_after/flux_scale plus per-band flux_correction_m/flux_correction_p rows), adds a NEO2_DISABLE_JOIN_LORENTZ_CORRECTION=1 env-var switch for a controlled A/B experiment that keeps uncorrected sources/extraction rows, and exports the Lorentz null-space orthogonal solution component in the qflux interface trace. Tests are updated to match the new recorder signature and to verify the new fields.

Findings:

  1. minor NEO-2-QL/join_diagnostics_mod.f90:29 — join_lorentz_correction_disabled()caches its result in module-levelsave variables (correction_switch_initialized, correction_switch_disabled`). The first call reads the env var; all later calls in the process return the same result. This is fine for a single run, but a caller cannot toggle the switch mid-run. No change required; just note the intended "read once" contract.

  2. minor TEST/test_join_failure_diagnostic.f90:65 — The test verifies the default-off state of the switch (.false. without the env var) and the non-finite-flux-scale rejection, but it never sets NEO2_DISABLE_JOIN_LORENTZ_CORRECTION=1 to exercise the disabled path (i.e., that source_p/source_m/flux_p/flux_m are left unmodified while the trace still records the would-be factors). Consider adding a test that asserts correction_disabled actually skips the subtraction (e.g., source_after == source_before). Not blocking.

  3. minor NEO-2-QL/qflux_profile_mod.f90:178 — The orthogonal component is computed as solution(column,force) - constant_coordinate(force)only form==0, where constant_coordinateis the block-wise mean of the solution over2*nbandentries. The math is correct and matches the test's expectedsolution(7,3) - sum(solution(5:8,3))/4`. Verified.

  4. minor NEO-2-QL/join_ends.f90:174 — correction_disabled = join_lorentz_correction_disabled()is called once perjoin_endsinvocation (insideIF(isw_lorentz.EQ.1), and the conditional subtraction is applied to both source_p/source_mandflux_p/flux_m`3
    No bugs; the conditional1
    8
    7
    7
    7
    7
    7
    7
    7
    7
    7
    7
    7
    7
    7
    3
    3
    7
    7
    7
    7

Verdict: Approve — The diff is well-structured, the new trace rows are validated for finiteness and positivity, tests are updated consistently with the new signatures, and the A/B switch is gated behind an env var with a loud audit line. The only suggestion is optional additional test coverage for the disabled-correction path.

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