Move DEVerbosity and default verbosity to DiffEqBase#3365
Conversation
|
This needs Project.toml bumps to be safe. |
|
Yes will do. The idea here is to move DEVerbosity to DiffEqBase, so that solvers that depend on just DiffEqBase rather than OrdinaryDiffEqCore can use DEVerbosity. Also, by having the default I'm working on the Sundials and ODEInterfaceDiffEq PRs now. |
jClugstor
force-pushed
the
move_verbosity
branch
from
ae597e7 to
8118c55
Compare
|
Sundials: ODEInterfaceDiffEq: |
|
@ChrisRackauckas this actually would break Sundials and ODEInterfaceDiffEq, since it changes the default I can try to do something different or find a workaround, or we can just put this in the breaking release? |
Aren't they already broken? The existing SciMLLogging changes already broke them for me: #3351 (comment) |
|
Sundials still works with a I mean that releasing this under a nonbreaking change would make code that uses Sundials with the default verbosity setting break. |
|
The current mix of SciMLogging changes and package releases makes it impossible to have a generic function call of Another question is also: will this be fixed in OrdinaryDiffEq < v7? Or will SciMLLogging continue to being broken on pre-v7 releases? |
|
All solvers will be supporting SciMLLogging and that should have perfectly fine inference, that's what this change is doing, and this either lands pre-v7 or gets backported. I'm hoping it just lands pre-v7 since backports will be a bit of a PITA |
These needs to get addressed before merging. |
|
@jClugstor you forgot DASSL.jl, IRKGaussLegendre.jl, DASKR.jl, ... |
|
IKRGaussLegendre: DASSL: DASKR: |
|
|
Right that's the breaking part from changing the default. That should fix the backwards compatibility. I'll also try to see if I can fix the inference for explicit Bool verbose, since I'm pretty sure that was working at some point. |
jClugstor
force-pushed
the
move_verbosity
branch
from
5110b6c to
e6d81d2
Compare
`DEFAULT_VERBOSE` and `NONE_VERBOSE` are different concrete
parameterizations of `DEVerbosity{...}`, so the ternary
`verbose ? DEFAULT_VERBOSE : NONE_VERBOSE` returns a `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`.
Annotate the `Bool` method with `Base.@constprop :aggressive` so the
literal `solve(...; verbose=true/false)` path collapses to a single
concrete type. Add `Val{true}`/`Val{false}` overloads as an explicit
type-stable entry point for callers that want to force inference
without relying on constprop.
Also add `[sources.DiffEqBase]` to `OrdinaryDiffEqMultirate/Project.toml`
so Pkg can pin 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.
Note: the deeper inference fix (Bool default flowing through multi-layer
kwargs splatting) is not solved here; `@inferred solve(...)` still fails
for the same reason it fails on SciML#3365's base. Changing the default to
`verbose = DEFAULT_VERBOSE` would fix it but breaks downstream solvers
like Sundials that still expect a `Bool`.
Co-Authored-By: Chris Rackauckas <accounts@chrisrackauckas.com>
|
Merged in a version with a constant prop fix. Downstream handling so everything uses the same DEVerbosity should all be merging ASAP as well. @devmotion just let me know if there's anything not handled here in the pre v7 DiffEqBase bump. |
3 participants
Checklist
contributor guidelines, in particular the SciML Style Guide and
COLPRAC.
Additional context
Add any other context about the problem here.