JuliaMath / QuadGK.jl

adaptive 1d numerical Gauss–Kronrod integration in Julia
MIT License
268 stars 37 forks source link

No cachedrule for DynamicQuantities types #88

Closed mikeingold closed 11 months ago

mikeingold commented 11 months ago

I just started experimenting with the recent DynamicQuantities.jl package, which aims to provide the same functionality as Unitful.jl but with a statically-typed Quantity to improve performance. It looks like QuadGK currently doesn't support these DynamicQuantities Quantity types, but I'm not familiar enough with the codebase to quite understand why.

I'm interested in contributing by generating a PR for this, but would really appreciate a nudge in the right direction. It looks like the issue spawns from approximately here, where either a @generated cachedrule isn't being produced at compile-time or maybe the generic cachedrule below it isn't working.

Any ideas that might help me understand what I can do to help integrate this functionality?

Thanks, Mike

Minimal reproduction (Julia v1.9.3):

julia> using DynamicQuantities  # [06fc5a27] DynamicQuantities v0.7.0

julia> using QuadGK  # [1fd47b50] QuadGK v2.8.2

julia> quadgk(t -> 5u"m/s"*t, 0u"s", 10u"s")
ERROR: MethodError: no method matching cachedrule(::Type{Quantity{Float64, Dimensions{DynamicQuantities.FixedRational{Int32, 25200}}}}, ::Int64)

Closest candidates are:
  cachedrule(::Union{Type{Complex{BigFloat}}, Type{BigFloat}}, ::Integer)
   @ QuadGK C:\Users\*\.julia\packages\QuadGK\XqIlh\src\gausskronrod.jl:253
  cachedrule(::Type{T}, ::Integer) where T<:Number
   @ QuadGK C:\Users\*\.julia\packages\QuadGK\XqIlh\src\gausskronrod.jl:264
stevengj commented 11 months ago

You need to provide a way to strip the units from the type. Currently, we use: https://github.com/JuliaMath/QuadGK.jl/blob/cb745d458495323414c27dfc33f8f77c60faf109/src/gausskronrod.jl#L571-L572

which assumes that one(T) returns the underlying dimensionless numeric type.

See e.g. https://github.com/JuliaPhysics/Measurements.jl/issues/134

mikeingold commented 11 months ago

Thanks! I'll see what I can do to get these packages working together.

stevengj commented 11 months ago

Closing in favor of https://github.com/SymbolicML/DynamicQuantities.jl/issues/40

mikeingold commented 11 months ago

Concur. Looks like this is due to DynamicQuantities’ current interpretation of the Base.one spec.