Pass the cache to nlsolve! in PDIRK44's out-of-place branch - #4352
Merged
ChrisRackauckas merged 1 commit intoAug 26, 2026
Conversation
singhharsh1708
force-pushed
the
pdirk-serial-threaded
branch
from
August 23, 2026 18:41
8b227ff to
cf50fc8
Compare
This was referenced Aug 23, 2026
singhharsh1708
force-pushed
the
pdirk-serial-threaded
branch
from
August 23, 2026 18:43
cf50fc8 to
363246e
Compare
This was referenced Aug 26, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
PDIRK44's out-of-place branch passesγ * dtwherenlsolve!takes theintegrator cache:
The signature is
nlsolve!(nlsolver, integrator, cache = nothing, repeat_step = false).Four call sites do this, all in that one branch. Every other one of the 74
nlsolve!calls in the library passes the cache, and the in-place branch of thesame function passes it correctly, so this looks like a slip rather than
something deliberate.
Nothing complained, for two reasons that compound. The guard
only rejects
nothing, and aFloat64sails through it. Thenupdate_W!on theout-of-place path routes to
calc_W(integrator, nlsolver, dtgamma, repeat_step),which does not read that argument at all. So the wrong value is carried down and
then ignored, and results are unaffected today.
This fixes the four call sites, and tightens the guard to check the type rather
than only
nothingso the next one of these is caught where it happens insteadof being carried silently.
Testing
lib/OrdinaryDiffEqPDIRK/test/nlsolve_argument_tests.jlchecks that the guardrejects a non-cache third argument, and that the out-of-place and in-place paths
agree to
rtol = 1e-10with threading both on and off. The guard test fails onthe current code and passes with this change; the agreement test passes either
way, since the old bug was silent, and is there to pin the behaviour.
OrdinaryDiffEqPDIRKCore andOrdinaryDiffEqNonlinearSolvesuites pass.One unrelated thing seen while running these, not touched here:
OrdinaryDiffEqPDIRK's QA group fails onSequential,BaseThreadsandPolyesterThreadsbeing reported as stale imports. They are imported only to benamed in an
Expr(:public, ...)declaration, which ExplicitImports does notcount as a use. It fails identically on an unmodified master, so it is not from
this change and wants its own decision about whether the
publicdeclaration orthe import should go.
Part of a set of three independent fixes in this area: #4345, #4351 and #4352.
They touch different code and do not overlap, but they bump some of the same
Project.tomlversion lines. Each bumps one patch from master rather thanreserving a number, since #4353 has just had to undo version skips for General's
AutoMerge, so whichever lands second needs its version line refreshed.
AI Disclosure
Claude assisted with this change.