Repository navigation
Give FIRK smooth_est = false an unfiltered error estimate - #4930
singhharsh1708 wants to merge 3 commits into
Conversation
Independent review (Codex CLI 0.163.0, model gpt-6.1-sol, reasoning effort high): CHANGES, risk medium-low (ML2). Reviewed head: Full reviewVERDICT: CHANGES Riskmedium-low (ML2: Error-estimate correction for non-default Radau configurations; default filtered estimates remain unchanged.) Risk assessment
Blocking findings
Non-blocking findingsNone. What I ranJulia 1.13.1, exclusively through the prescribed wrapper. Commands below use worktree-relative executable and probe paths. Git inspection commands exited 0: git rev-parse HEAD
git merge-base origin/master HEAD
git diff origin/master...HEAD
git log -1 --format=%B
git status --short
git diff --checkHEAD and merge-base matched the specified SHAs. The latest commit has no Read linked issue #4912 with: gh issue view 4912 --repo SciML/OrdinaryDiffEq.jl --json number,title,body,urlExit 0. Ran the changed test file on head: ../../jrun --project=lib/OrdinaryDiffEqFIRK -e 'using TestEnv; TestEnv.activate(); using Test, SafeTestsets; println("Julia ", VERSION); include("lib/OrdinaryDiffEqFIRK/test/ode_firk_tests.jl")'Exit 0. Output tail: Copied the PR’s changed Failures reproduced Ran an independent analytic probe on head: ../../jrun --project=lib/OrdinaryDiffEqFIRK -e 'using TestEnv; TestEnv.activate(); include("../../tmp/SciML__OrdinaryDiffEq.jl__4930/analytic_probe.jl")'Exit 1: All six identity-matrix cases succeeded, with maximum endpoint error 8.44e-9. Initial Used the second short probe on base to attribute these failures: ../../jrun --project=lib/OrdinaryDiffEqFIRK -e 'using TestEnv; TestEnv.activate(); include("../../tmp/SciML__OrdinaryDiffEq.jl__4930/baseline_probe.jl")'Exit 0: Thus neither exploratory failure is attributed to this PR. Inspected supplied FIRK CI results, PR CI failures, and downstream failures. Compared against recent master CI and master downstream logs: the ReverseDiff tape errors, stiff-adjoint unexpected passes, ensemble assertion, and GPUCompiler warning failure have matching signatures. What I did not verify
AUTHOR_GRADE: {"task":"bugfix","score":3,"difficulty":3,"rounds":1,"author_harness":"unknown","author_model":"unknown","note":"The regression fails on base and passes on head, and independent reviewer probes support the identity-matrix correction. The PR still needs a retained independent accuracy oracle for its numerical change; repairing solver error estimation is a moderate numerical task."} 🤖 Posted by an AI agent · harness: Claude Code · model: claude-opus-5-5 (coordinating gpt-6.1-sol reviewers) |
1bab25d to
7a50a3a
Compare
With
smooth_est = falseand an identity mass matrix,RadauIIA5,RadauIIA9andAdaptiveRadaunow estimate the error as-(dt/γ) (f₀ + Σ eᵢzᵢ/dt), the limit of the smoothed estimateW1⁻¹ (f₀ + M Σ eᵢzᵢ/dt)for smalldt J, withW1 = J - (γ/dt) M. With any other mass matrix that limit needsM⁻¹, which a singularMdoes not have, so the smoothed estimate is used; using the identity formula there lost error control (maximum error 5.3e-3 at tolerance 1e-6 withM = 1e-4 I).Before, the flag only skipped the
W1solve: in placeutildestayed the transformed stagew2, and out of place it stayedf₀ + M Σ eᵢzᵢ/dt, a rate aboutγ/dttimes too large. The same applies in the first-step retry branch, where the estimate is added touprev.On the pulse problem from the issue, master takes 88,779 to 182,216 steps or stops (MaxIters, Unstable) in every
smooth_est = falsecase; with this change all twelve finish in 32 to 175 steps, within one step of the default, with relative errors between 7.3e-14 and 1.7e-8. Runs withsmooth_est = trueare identical to master.The new testset (
u' = -50 (u - cos t)) fails on master: in place the unsmoothed runs hit MaxIters, and out of place RadauIIA5 takes 357 steps against 47. It also checks that after one step atdt = 0.01the unsmoothed estimate is within 2% of the smoothed one (a copy withγdropped at all twelve sites fails it), and that with a scaled diagonal mass matrixsmooth_est = falsegives the smoothed solve exactly. FIRK Core passes.The
smooth_estdocstring, "TBD" until now, describes both settings.RadauIIA3, which has nosmooth_estkeyword, now gets its own keyword list without it.Part of #4912; the
extrapolanthalf is #4924.AI Disclosure: Claude assisted with this change.