Make _process_verbose_param(::Bool) constprop-friendly to fix @inferred solve#3438
Merged
ChrisRackauckas merged 9 commits intoApr 15, 2026
Conversation
e43ab1d to
bf421cf
Compare
342b718 to
32f75c5
Compare
Member
|
Testing the MWE in #3351 with this change, I still get that the inferred return type is Putting a function barrier like : _verbose = verbose isa Bool ? (verbose ? Standard() : None()) : verbose
return _ode_init(prob, alg, timeseries_init, ts_init, ks_init; verbose = _verbose, kwargs...)in I haven't found a way to make the Bool path fully infer yet. |
32f75c5 to
1fc8763
Compare
`DEFAULT_VERBOSE` and `NONE_VERBOSE` are different concrete
parameterizations of `DEVerbosity{...}`, so the ternary
`verbose ? DEFAULT_VERBOSE : NONE_VERBOSE` returns `Union{...}` whenever
`verbose::Bool` isn't a compile-time constant. That `Union` propagates
into solver caches (e.g. baked into `LinearCache` via `LinearVerbosity`)
and breaks return-type inference of `solve`, causing `@inferred
solve(...)` failures across the Rosenbrock/Default/BDF/SDIRK test suites.
Simply annotating `_process_verbose_param(::Bool)` with `Base.@constprop
:aggressive` isn't enough, because the `verbose = true` default at the
`DiffEqBase.solve` entrypoint has to thread through several layers of
kwargs splatting (`solve` → `solve_up` → `__solve` → `__init` →
`_ode_init`) before reaching it, and constprop gives up at any
unannotated layer. Annotate each layer with `Base.@constprop :aggressive`
so the Bool literal propagates end-to-end. Also add `Val{true}`/
`Val{false}` overloads for callers that want to force type-stability
without relying on constprop at all.
Also add `[sources.DiffEqBase]` to `OrdinaryDiffEqMultirate/Project.toml`
so Pkg pins the local `DiffEqBase 6.217` alongside the other sublibs
during its tests — otherwise the resolver only sees the registered
`6.216` which violates `OrdinaryDiffEqCore 3.32`'s new `DiffEqBase ≥
6.217` compat and the test environment fails to instantiate.
Verified locally: `@inferred solve(prob_mm, Rodas5P(), reltol=1.0e-8,
abstol=1.0e-8)` passes on this branch (failed without the annotations).
Co-Authored-By: Chris Rackauckas <accounts@chrisrackauckas.com>
1fc8763 to
3e0d99e
Compare
Member
|
This at least makes constant prop work which I think is good enough for the pre-v7 if verbosity is fully supported downstream, because then the defaults and the suggested ways to set |
2 tasks
singhharsh1708
pushed a commit
to singhharsh1708/OrdinaryDiffEq.jl
that referenced
this pull request
Apr 16, 2026
… solve (SciML#3438) * raise DEVerbosity to DiffEqBase * add toggle for Sundials * bump versions * bump versions * use DiffEqBase for DEVerbosity access * bump lower bound for DiffEqBase in DelayDiffEq * change default verbosity back to true * remove unnecessary function extensions * Fix @inferred solve via constprop on the verbose kwarg chain `DEFAULT_VERBOSE` and `NONE_VERBOSE` are different concrete parameterizations of `DEVerbosity{...}`, so the ternary `verbose ? DEFAULT_VERBOSE : NONE_VERBOSE` returns `Union{...}` whenever `verbose::Bool` isn't a compile-time constant. That `Union` propagates into solver caches (e.g. baked into `LinearCache` via `LinearVerbosity`) and breaks return-type inference of `solve`, causing `@inferred solve(...)` failures across the Rosenbrock/Default/BDF/SDIRK test suites. Simply annotating `_process_verbose_param(::Bool)` with `Base.@constprop :aggressive` isn't enough, because the `verbose = true` default at the `DiffEqBase.solve` entrypoint has to thread through several layers of kwargs splatting (`solve` → `solve_up` → `__solve` → `__init` → `_ode_init`) before reaching it, and constprop gives up at any unannotated layer. Annotate each layer with `Base.@constprop :aggressive` so the Bool literal propagates end-to-end. Also add `Val{true}`/ `Val{false}` overloads for callers that want to force type-stability without relying on constprop at all. Also add `[sources.DiffEqBase]` to `OrdinaryDiffEqMultirate/Project.toml` so Pkg pins the local `DiffEqBase 6.217` alongside the other sublibs during its tests — otherwise the resolver only sees the registered `6.216` which violates `OrdinaryDiffEqCore 3.32`'s new `DiffEqBase ≥ 6.217` compat and the test environment fails to instantiate. Verified locally: `@inferred solve(prob_mm, Rodas5P(), reltol=1.0e-8, abstol=1.0e-8)` passes on this branch (failed without the annotations). Co-Authored-By: Chris Rackauckas <accounts@chrisrackauckas.com> --------- Co-authored-by: jClugstor <jadonclugston@gmail.com> Co-authored-by: ChrisRackauckas-Claude <accounts@chrisrackauckas.com>
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.
Depends on #3365 — this branch is
move_verbosity+ one extra commit. Once #3365 merges, only the new commit (Make _process_verbose_param(::Bool) constprop-friendly) will remain on the diff against master. If preferred, this can be merged into #3365's branch first; mirror PR was at jClugstor#2 (closed in favor of this).Summary
DEFAULT_VERBOSEandNONE_VERBOSEare different concrete parameterizations ofDEVerbosity{...}(differentSilent/WarnLevel/InfoLeveltype parameters per toggle), so the ternaryverbose ? DEFAULT_VERBOSE : NONE_VERBOSEreturns aUnion{...}wheneververbose::Boolisn't a compile-time constant.Unionflows into the solver caches — you can seeLinearVerbosity{Silent,Silent,...,WarnLevel,WarnLevel,...}baked into theLinearCachetype in Move DEVerbosity and default verbosity to DiffEqBase #3365's CI failure logs — and it poisons return-type inference all the way toODESolution. Tests like@inferred solve(prob_mm, Rodas5P(), ...)inlib/OrdinaryDiffEqRosenbrock/test/dae_rosenbrock_ad_tests.jlfail withdoes not match inferred return type Any.Boolmethod withBase.@constprop :aggressiveso the compiler threads the literal defaultverbose = truethrough the kwargs chain to a single concreteDEVerbosityparameterization.Val{true}/Val{false}overloads as an explicit type-stable entry point for callers that don't want to rely on constprop reaching them.The runtime-
Bool-with-no-constprop case still returns aUnion; that's an inherent property of the@verbosity_specifierdesign (verbosity levels are encoded as type parameters). The proper fix for that case is to keep verbosity out of the cache type parameters, but this is the smallest change that unblocks #3365's@inferredtests.Test plan
lib/DiffEqBase/test/verbose_inference.jlexercises both theVal-dispatch path (unconditionally inferable) and the literal-Boolconstprop path (verifiesBase.return_typesresolves to a single concrete type).@inferred solve(...)errors.