ModelingToolkit.jl icon indicating copy to clipboard operation
ModelingToolkit.jl copied to clipboard

Unit checking error message

Open lamorton opened this issue 2 years ago • 2 comments

This term should evaluate to have "units" of [MPA * mm^γ] but right now this is failing.

using ModelingToolkit, Unitful
pars = @parameters begin
    Q, [unit = u"MPa"],
    γ,
    r, [unit = u"mm"]
end

bad_term =  Q * r^γ
MT.get_unit(bad_term)

produces

ERROR: ModelingToolkit.ValidationError("Unable to get unit for operation * with arguments SymbolicUtils.Symbolic{Real}[Q, r^γ].")
Stacktrace:
 [1] get_unit(op::Function, args::Vector{SymbolicUtils.Symbolic{Real}})
   @ ModelingToolkit ~/.julia/packages/ModelingToolkit/H0KaK/src/systems/validation.jl:55
 [2] get_unit(x::SymbolicUtils.Mul{Real, Int64, Dict{Any, Number}, Nothing})
   @ ModelingToolkit ~/.julia/packages/ModelingToolkit/H0KaK/src/systems/validation.jl:127
 [3] get_unit(x::Num)
   @ ModelingToolkit ~/.julia/packages/ModelingToolkit/H0KaK/src/systems/validation.jl:43
 [4] top-level scope
   @ Untitled-1:9

caused by: MethodError: no method matching unit(::SymbolicUtils.Mul{Quantity{Real}, Quantity{Int64, 𝐌 𝐋⁻¹ 𝐓⁻², Unitful.FreeUnits{(MPa,), 𝐌 𝐋⁻¹ 𝐓⁻², nothing}}, Dict{Any, Number}, Nothing})
Closest candidates are:
  unit(::Union{Dates.Day, Dates.Hour, Dates.Microsecond, Dates.Millisecond, Dates.Minute, Dates.Nanosecond, Dates.Second, Dates.Week}) at ~/.julia/packages/Unitful/SUQzL/src/dates.jl:31
  unit(::Unitful.MixedUnits{L, U}) where {L, U} at ~/.julia/packages/Unitful/SUQzL/src/logarithm.jl:98
  unit(::Unitful.AbstractQuantity{T, D, U}) where {T, D, U} at ~/.julia/packages/Unitful/SUQzL/src/utils.jl:109
  ...
Stacktrace:
 [1] get_unit(op::Function, args::Vector{SymbolicUtils.Symbolic{Real}})
   @ ModelingToolkit ~/.julia/packages/ModelingToolkit/H0KaK/src/systems/validation.jl:53
 [2] get_unit(x::SymbolicUtils.Mul{Real, Int64, Dict{Any, Number}, Nothing})
   @ ModelingToolkit ~/.julia/packages/ModelingToolkit/H0KaK/src/systems/validation.jl:127
 [3] get_unit(x::Num)
   @ ModelingToolkit ~/.julia/packages/ModelingToolkit/H0KaK/src/systems/validation.jl:43
 [4] top-level scope
   @ Untitled-1:9

There's a test for handling exponentiation:

@test MT.equivalent(MT.get_unit(P^γ), MT.get_unit((E / τ)^γ))

It looks like what's happening here is related to the way the * operator is hitting the fallback method get_unit(op, args) which just tries to run the operation & get the resulting units. We may want to specialize get_unit(op::Mul,args) and maybe ditch this fallback method.

lamorton avatar Sep 08 '22 17:09 lamorton

Agreed, this sounds like a nice potential improvement.

ChrisRackauckas avatar Sep 08 '22 21:09 ChrisRackauckas

Related bug w/ the fallback:

@parameters d [unit = u"cm"] r [unit = u"mm"]
MT.get_unit(log(d/r)) 

This yields dimensionless units for the term, but it should throw a validation error b/c the argument to the log needs to be dimensionless. The fallback cheats by running thelog function on d/r which allows Unitful to cope with the mismatched units, but that won't happen in the generated function (sans units) so this allows incorrect behavior.

lamorton avatar Sep 09 '22 15:09 lamorton

Same issue as https://github.com/SciML/ModelingToolkit.jl/issues/2339, will continue in that one.

ChrisRackauckas avatar Feb 22 '24 16:02 ChrisRackauckas