Fix DQ extension: wire up value / stripunits / calculate_residuals for Quantity#3493
Open
ChrisRackauckas-Claude wants to merge 1 commit into
Conversation
The new DynamicQuantities+Measurements downstream test (abfbbd6) failed the Tsit5 solve because `DiffEqBaseDynamicQuantitiesExt` didn't cover enough of DiffEqBase's wrapper-stripping helpers. SciML#3485's 8a3ffd5 patched solve.jl to call `DiffEqBase.value` and `DiffEqBase.stripunits`, but the corresponding DQ overrides were never added — each helper fell back to identity on `Quantity`. Two gaps fixed in `DiffEqBaseDynamicQuantitiesExt.jl`: 1. `SciMLBase.value` / `SciMLBase.unitfulvalue` — parallel to the Unitful and FlexUnits extensions. `value(::Quantity) = value(ustrip(x))` recurses so both the DQ wrapper and any inner Measurement/Dual are stripped. Without this, `initdt.jl:320`'s `eps(DiffEqBase.value(t))` errored with `MethodError: no method matching eps(::Quantity{Float64, Dimensions})`. 2. `RecursiveArrayTools.recursive_unitless_bottom_eltype` / `recursive_unitless_eltype` — the upstream generic `Number` dispatch uses `typeof(one(T))`, and DynamicQuantities' `one(::Type{<:Quantity})` returns a dimensionless `Quantity{Float64, D}`, so units survived. Recurse into `DynamicQuantities.value_type(T)` instead so `uBottomEltypeNoUnits` lands on a bare scalar type and the `zero(uBottomEltypeNoUnits)` at `solve.jl:616` (the originally reported failure) works. A third failure — `@fastmath ũ / …` in `calculate_residuals` — is an upstream DynamicQuantities gap (`Base.FastMath.div_fast` promotes before dividing, and there's no promote rule between `Quantity{Measurement,D}` and a bare `Measurement`). Filed separately against DynamicQuantities.jl; the `dynamicquantities_measurements` downstream test will remain errored in the meantime. Failing CI that motivated this: https://github.com/SciML/OrdinaryDiffEq.jl/actions/runs/24748633476/job/72406481107 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Co-Authored-By: Chris Rackauckas <accounts@chrisrackauckas.com>
1ff9123 to
b6d4029
Compare
2 tasks
MilesCranmer
pushed a commit
to JuliaPhysics/DynamicQuantities.jl
that referenced
this pull request
Apr 22, 2026
…low 4 and 3 (#214) `DynamicQuantitiesRecursiveArrayToolsExt` and `DynamicQuantitiesSciMLBaseExt` were added in #201 with `RecursiveArrayTools = "3"` / `SciMLBase = "2"` in compat. The SciML ecosystem has since moved to RecursiveArrayTools 4 and SciMLBase 3, which forces Pkg to resolve DynamicQuantities back to v1.11.0 (the last version without the weak-dep constraint) in any environment pulling recent DifferentialEquations. That in turn means neither extension loads and downstream solvers trip over `zero(::Type{<:Quantity})` / `recursive_unitless_bottom_eltype(::Type{<:Quantity})` returning a still-unitful type. The extensions touch only `SciMLBase.value` / `unitfulvalue` and `RecursiveArrayTools.recursive_unitless_(bottom_)eltype` — all stable APIs across these major versions. Verified: `test/test_sciml.jl` passes on a fork that pulls `RecursiveArrayTools v4.2.0` and `SciMLBase v3.3.0`. This lets downstream packages (OrdinaryDiffEq.jl in particular — see SciML/OrdinaryDiffEq.jl#3493) drop their in-tree DQ-wrapper-stripping workarounds once a DQ patch with this bump is released. Co-authored-by: ChrisRackauckas-Claude <accounts@chrisrackauckas.com> Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.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.
Summary
Supersedes #3492 (which I closed with a "just delete the test" patch; this fixes the underlying bugs instead).
The new DynamicQuantities+Measurements downstream test (abfbbd6) failed the
Tsit5solve at three separate points, all caused byDiffEqBaseDynamicQuantitiesExtnot covering enough of DiffEqBase's wrapper-stripping surface. #3485's 8a3ffd5 patchedsolve.jlinit to callDiffEqBase.value/DiffEqBase.stripunits, but the corresponding DQ overrides were never added — each helper fell back to identity onQuantity.Three bugs fixed
SciMLBase.value/SciMLBase.unitfulvalue— parallel to the Unitful and FlexUnits extensions.value(::Quantity) = value(ustrip(x))recurses so both the DQ wrapper and any innerMeasurement/Dualare stripped. Without it,initdt.jl:320'seps(DiffEqBase.value(t))errored with:RecursiveArrayTools.recursive_unitless_bottom_eltype/recursive_unitless_eltype— the upstream genericNumberdispatch usestypeof(one(T)), and DynamicQuantities'one(::Type{<:Quantity})returns a dimensionlessQuantity{Float64, D}, so units survived the "unitless" transformation. Recurse intoDynamicQuantities.value_type(T)instead souBottomEltypeNoUnitsbecomes a bare scalar type andzero(uBottomEltypeNoUnits)atsolve.jl:616— the originally reported failure — lands on a validMeasurement{Float64}.DiffEqBase.calculate_residuals— the scalar methods use@fastmath ũ / (α + …), which routes throughBase.FastMath.div_fastandpromotes its args. Promotion ofQuantity{Measurement{Float64}, D}against a bareMeasurement{Float64}tolerance fails ("promotion of types … failed to change any arguments"). Specialize forũ::UnionAbstractQuantity(and the 5-arg(u₀, u₁, …)variant) to use plain/.Each override is narrow — it only intercepts calls whose argument carries DQ units, so non-DQ code paths are untouched.
Verification
Ran the exact downstream test locally against the patched extension (dev-ed DiffEqBase, OrdinaryDiffEqCore, OrdinaryDiffEqTsit5 into a fresh project):
Failing CI that motivated this:
https://github.com/SciML/OrdinaryDiffEq.jl/actions/runs/24748633476/job/72406481107
Test plan
dynamicquantities_measurements.jland it passes (4 asserts)