Repository navigation
Reuse in-place FIRK W only at the dt it was factored for - #4941
singhharsh1708 wants to merge 1 commit into
Conversation
|
This is going to substantially slow down the method. |
|
@oscardssmith Measured against master, in-place, reltol 1e-9, abstol 1e-12. Master refactors |
Independent review (Codex CLI 0.163.0, model gpt-6.1-sol, reasoning effort high): MERGE-WITH-ISSUES, risk medium (M2). Reviewed head: Full reviewVERDICT: MERGE-WITH-ISSUES Riskmedium (M2: Factorization reuse changes numerical results on existing in-place Radau paths, beyond rounding differences.) Risk assessment
Blocking findingsNone. Non-blocking findings
What I ranLocation aliases below represent the supplied wrapper ( Read-only git checks, all exit 0: git rev-parse HEAD
git diff origin/master...HEAD
git merge-base origin/master HEAD
git log -1 --no-merges --format=%B
git diff --check origin/master...HEAD
git status --short
git -C "$BASE_WORKTREE" rev-parse HEAD
git -C "$BASE_WORKTREE" diff --name-onlyThe head and merge-base SHAs matched the supplied revisions. The head remained clean; base differed only by the authorized test-file copy. No agent trailers were present. Copied the changed test file into base and verified identical contents; both commands exited 0: cp lib/OrdinaryDiffEqFIRK/test/ode_firk_tests.jl "$BASE_WORKTREE/lib/OrdinaryDiffEqFIRK/test/ode_firk_tests.jl"
cmp lib/OrdinaryDiffEqFIRK/test/ode_firk_tests.jl "$BASE_WORKTREE/lib/OrdinaryDiffEqFIRK/test/ode_firk_tests.jl"Ran this exact Julia command separately from head and base: "$JRUN" --project=lib/OrdinaryDiffEqFIRK -e 'using TestEnv; TestEnv.activate(); using Test, SafeTestsets; include("lib/OrdinaryDiffEqFIRK/test/ode_firk_tests.jl")'
Ran the diagnostic script separately from head and base: "$JRUN" --project=lib/OrdinaryDiffEqFIRK "$PROBE"The script used the analytic solution (u(t)=\sin(\omega t)) for
All Julia execution used the supplied wrapper: Julia 1.13.1, four threads, one BLAS thread, and serialized execution. Inspected the supplied CI snapshots and failure excerpts:
What I did not verify
AUTHOR_GRADE: {"task":"bugfix","score":4,"difficulty":3,"rounds":1,"author_harness":"unknown","author_model":"unknown","note":"The analytic callback regression fails on base and passes on head, and targeted numerical probes retain accuracy. Minor follow-ups are order-change cache invalidation to avoid a recoverable failed attempt and explicit completion assertions; the task requires moderate solver-cache reasoning."} 🤖 Posted by an AI agent · harness: Claude Code · model: claude-opus-5-5 (coordinating gpt-6.1-sol reviewers) |
In-place
RadauIIA3,RadauIIA5,RadauIIA9andAdaptiveRadaunow reuse a factoredWonly at thedtit was built for. They reused it for a step up to 20% away (new_W_γdt_cutoff = 1//5) while still accepting the first Newton iterate on the previous step's convergence rate, so a step with a staleWcould be accepted after one iteration with its error unresolved.cache.W_γdtwas only updated with a new Jacobian, so in practice reuse stayed off until something forced one: any callback that fires and leavesu_modifiedset (it marks a derivative discontinuity), or ad_discontinuitiesentry once #4932 lands.W_γdtis now recorded wheneverWis rebuilt, and the defaultnew_W_γdt_cutofffor these four methods is 0, soWis kept only while the controller holdsdtfixed, which is how RADAU5 reuses its decomposition. Out of place,Wis built every step and nothing changes.On
u' = -3u + 1with a no-opDiscreteCallbackat t = 0.5 (reltol 1e-8), master's in-place RadauIIA5, RadauIIA9 and AdaptiveRadau end 68, 779 and 736 times reltol off at their own steps; here they match the run without the callback (0.031, 0.0008 and 0.0026). With #4932 applied, a drive switching at 0.5:0.5:2.5 given asd_discontinuitiesgoes from up to 1.6e4 times reltol to below 0.04. On Van der Pol (mu = 1e3), OREGO and ROBER at 1e-6 and 1e-9, RadauIIA5 needs fewer factorizations for the same steps (Van der Pol 1e-9: 2030 to 720; OREGO 1e-9: 1227 to 814). Final errors are mostly smaller (Van der Pol RadauIIA5 1e-9: 8.3 to 0.23 times tolerance; OREGO 1e-6: 13 to 0.7) but not everywhere (ROBER RadauIIA9 1e-9: 8 to 19), and AdaptiveRadau on Van der Pol at 1e-9 takes 627 steps against 302.The new testset is the callback problem; it fails on master for all three methods. FIRK Core and QA pass, as do the NonlinearSolve mass-matrix tests (which the Core group never reaches on master, see #4681) and the root Integrators_I, InterfaceI, InterfaceIII, InterfaceIV and Regression_II groups.
RadauIIA3is left out of the test: after a discontinuity its error is 2.6e-5 at every tolerance, on master too and out of place as well, which is a separate problem.FIRK goes to 2.8.10, the same version as the other open FIRK PRs.
AI Disclosure: Claude assisted with this change.