Skip to content

Fix DQ extension: wire up value / stripunits / calculate_residuals for Quantity#3493

Open
ChrisRackauckas-Claude wants to merge 1 commit into
SciML:masterfrom
ChrisRackauckas-Claude:fix-downstream-dynamicquantities-measurements
Open

Fix DQ extension: wire up value / stripunits / calculate_residuals for Quantity#3493
ChrisRackauckas-Claude wants to merge 1 commit into
SciML:masterfrom
ChrisRackauckas-Claude:fix-downstream-dynamicquantities-measurements

Conversation

@ChrisRackauckas-Claude

Copy link
Copy Markdown
Contributor

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 Tsit5 solve at three separate points, all caused by DiffEqBaseDynamicQuantitiesExt not covering enough of DiffEqBase's wrapper-stripping surface. #3485's 8a3ffd5 patched solve.jl init to call DiffEqBase.value / DiffEqBase.stripunits, but the corresponding DQ overrides were never added — each helper fell back to identity on Quantity.

Three bugs fixed

  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 it, initdt.jl:320's eps(DiffEqBase.value(t)) errored with:

    MethodError: no method matching eps(::Quantity{Float64, Dimensions{FRInt32}})

  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 the "unitless" transformation. Recurse into DynamicQuantities.value_type(T) instead so uBottomEltypeNoUnits becomes a bare scalar type and zero(uBottomEltypeNoUnits) at solve.jl:616 — the originally reported failure — lands on a valid Measurement{Float64}.

    Cannot create an additive identity from Type{<:DynamicQuantities.Quantity}

  3. DiffEqBase.calculate_residuals — the scalar methods use @fastmath ũ / (α + …), which routes through Base.FastMath.div_fast and promotes its args. Promotion of Quantity{Measurement{Float64}, D} against a bare Measurement{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):

Test Summary:                                      | Pass  Total  Time
DynamicQuantities units + Measurements uncertainty |    4      4  5.4s

Failing CI that motivated this:
https://github.com/SciML/OrdinaryDiffEq.jl/actions/runs/24748633476/job/72406481107

Test plan

  • CI Downstream group runs dynamicquantities_measurements.jl and it passes (4 asserts)
  • Other downstream tests (Measurements-only, time-derivative) continue to pass — nothing touched on the non-DQ path

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>
@ChrisRackauckas-Claude ChrisRackauckas-Claude force-pushed the fix-downstream-dynamicquantities-measurements branch from 1ff9123 to b6d4029 Compare April 22, 2026 05:16
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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants