Skip to content

ForceFreeStates - REFACTOR - ⏸️ Single-source the auto chunk-count target - #435

Draft
d-burg wants to merge 2 commits into
feature/thread-invariancefrom
refactor/single-source-chunk-target
Draft

d-burg wants to merge 2 commits into
feature/thread-invariancefrom
refactor/single-source-chunk-target

Conversation

@d-burg

@d-burg d-burg commented Aug 28, 2026 •

Copy link
Copy Markdown
Collaborator

Release note

  • Audience: developers
  • Numerical impact: none (harness @ a47f409) — the resolved chunk targets are unchanged; diiid_n1_riccati is bit-identical head vs base
  • Migration: none

The nchunks = 0 auto chunk-count target and the crossing floor were each written out in three places: the authoritative formula inside balance_integration_chunks plus hand-copied mirrors in runtests_parallel_integration.jl and runtests_decomposition_invariance.jl. They are now single functions — ForceFreeStates.auto_chunk_target(msing) and ForceFreeStates.min_crossing_chunks(msing) — used by the solver and both test files, and the struct docstring, docs/src/stability.md and the test comments now point at them instead of restating the formula.

Regression report

regress --cases diiid_n1_riccati --refs 30eb52073,a47f409f0 --force (base = #362's head feature/thread-invariance, so the comparison isolates exactly this refactor; raw shas, both sides fresh):

Regression Report: diiid_n1_riccati
Ref 1: 30eb52073  @ 30eb52073 (2026-08-28)
Ref 2: a47f409f0  @ a47f409f0 (2026-08-28)
Quantity                       30eb52073      a47f409f0      Diff     Status
delta prime (BVP diagonal)     [5 elem]       [5 elem]       0.0e+00  OK
delta prime (raw side-major)   [10 elem]      [10 elem]      0.0e+00  OK
edge coil response delta_coil  [10 elem]      [10 elem]      0.0e+00  OK
total energy Re(et[1])         8.038607e-01   8.038607e-01   0.0e+00  OK
plasma energy Re(ep[1])        -1.344628e+00  -1.344628e+00  0.0e+00  OK
vacuum energy Re(ev[1])        2.148488e+00   2.148488e+00   0.0e+00  OK
total energy (all)             [35 elem]      [35 elem]      0.0e+00  OK
# singular surfaces            5              5              0.0e+00  OK
singular psi locations         [5 elem]       [5 elem]       0.0e+00  OK
singular q values              [5 elem]       [5 elem]       0.0e+00  OK
ODE steps (saved)              51             51             0.0e+00  OK
ODE steps (total)              1667           1667           0.0e+00  OK
mpert                          35             35             0.0e+00  OK
npert                          1              1              0.0e+00  OK
q0                             1.204212e+00   1.204212e+00   0.0e+00  OK
q95                            4.781723e+00   4.781723e+00   0.0e+00  OK
beta_n                         1.372511e+00   1.372511e+00   0.0e+00  OK
Runtime (s)                    107.8s         107.8s                  --
Summary: 17 unchanged

Validation

Notes for reviewers

  • The formula is pinned by value in runtests_parallel_integration.jl: auto_chunk_target.((0, 1, 5)) == (8, 17, 53) and min_crossing_chunks.((0, 1, 5)) == (3, 5, 13). A change to the src formula fails that test and has to update a spec, rather than being pasted into a copy of the expression. (The harness diiid_n1_riccati case would also catch it through the chunk list.)
  • auto_chunk_target drops the max(2m+3, …) from the old expression: 9m+8 > 2m+3 for every m ≥ 0, so the floor could never win. Value-identical for every msing.
  • The min_bvp_intervals conditioning rationale moved from the inline comment into auto_chunk_target's docstring, which also corrects the old comment's stale "at least 5 non-crossing sub-chunks" to the 8 the code uses.
  • Tests after these edits (b7ace7893): runtests_parallel_integration.jl 96/96.
  • Stacked on Test - TEST - ⏸️ Decomposition invariance of the unified Riccati Delta-prime path #362 (feature/thread-invariance), since runtests_decomposition_invariance.jl only exists there; retarget to develop after Test - TEST - ⏸️ Decomposition invariance of the unified Riccati Delta-prime path #362 merges.

Draft until #362 lands and Daniel names reviewer/assignee.

🤖 Generated with Claude Code

The nchunks=0 target and the crossing floor were mirrored by hand in two
test files. They are now min_crossing_chunks(msing) and
auto_chunk_target(msing), used by balance_integration_chunks and both
tests; one deliberate assertion pins the formula so a src change still
fails loudly instead of propagating silently.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@github-actions github-actions Bot added the refactor Behavior-preserving restructuring label Aug 28, 2026
@github-actions

Copy link
Copy Markdown
Contributor

This pull request is missing an assignee and a reviewer.

If you are not ready to name them, mark this pull request as a draft.
docs/development/contributors.md suggests lead developers to ask.
Merging is not blocked here, but no pull request may be merged without human review.

… pin it by value

- The struct docstring, docs/src/stability.md and the test comment now point at
  auto_chunk_target / min_crossing_chunks instead of restating the formula.
- The test pins auto_chunk_target and min_crossing_chunks by value (a spec), not by a copy of
  the expression.
- auto_chunk_target drops a max whose floor could never win (9m+8 > 2m+3 for m ≥ 0);
  value-identical for every msing.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@logan-nc

logan-nc commented Oct 3, 2026

Copy link
Copy Markdown
Collaborator

@d-burg should this be opened and assigned a reviewer?

@d-burg d-burg changed the title ForceFreeStates - REFACTOR - Single-source the auto chunk-count target ForceFreeStates - REFACTOR - ⏸️ Single-source the auto chunk-count target Oct 5, 2026

This branch has not been deployed

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

Labels

refactor Behavior-preserving restructuring

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants