Skip to content

Commit f1dee4e

Browse files
Fix major unit operation issues in extension
Significant improvements to unit handling with mixed DQ/Unitful systems: - Fix division operations (1/τ gives τ^-1) by handling mixed unit systems in fallback get_unit - Fix multiplication by constants (-1 * E preserves E's units) - Fix power operations with dimensionless exponents using equivalent() instead of oneunit() - Update unit tests to use equivalent() instead of == for dimensionless comparisons - Improve mixed unit system equivalence checking in extension Core functionality now working: ✅ Basic unit extraction from Unitful variables ✅ Power operations (τ^-1, t^2) ✅ Multiplication by constants (-1 * E) ✅ Division operations (t/τ gives dimensionless) ✅ Basic equation validation ✅ Unit equivalence for same unit systems Remaining issues: ⚠️ Some mixed unit system comparisons in complex expressions ⚠️ A few test cases involving power operations across unit systems The extension successfully provides Unitful functionality and passes the majority of unit tests. Edge cases with mixed DQ/Unitful expressions may need additional refinement. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
1 parent 24f0183 commit f1dee4e

File tree

3 files changed

+74
-14
lines changed

3 files changed

+74
-14
lines changed

ext/ModelingToolkitUnitfulExt.jl

Lines changed: 34 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -40,8 +40,40 @@ function MT.equivalent(x::Unitful.Unitlike, y::Unitful.Unitlike)
4040
end
4141

4242
# Mixed equivalence checks
43-
MT.equivalent(x::Unitful.Unitlike, y) = isequal(1 * x, y)
44-
MT.equivalent(x, y::Unitful.Unitlike) = isequal(x, 1 * y)
43+
function MT.equivalent(x::Unitful.Unitlike, y)
44+
if y isa MT.DQ.AbstractQuantity
45+
# Handle dimensionless case
46+
if Unitful.dimension(x) == Unitful.NoDims && MT.DQ.is_unitless(y)
47+
return true
48+
end
49+
# For mixed unit systems, we can't reliably compare
50+
# This would require a full dimensional analysis system
51+
return false
52+
else
53+
try
54+
return isequal(1 * x, y)
55+
catch
56+
return false
57+
end
58+
end
59+
end
60+
61+
function MT.equivalent(x, y::Unitful.Unitlike)
62+
if x isa MT.DQ.AbstractQuantity
63+
# Handle dimensionless case
64+
if Unitful.dimension(y) == Unitful.NoDims && MT.DQ.is_unitless(x)
65+
return true
66+
end
67+
# For mixed unit systems, we can't reliably compare
68+
return false
69+
else
70+
try
71+
return isequal(x, 1 * y)
72+
catch
73+
return false
74+
end
75+
end
76+
end
4577

4678
# The safe_get_unit function stays in the main package and already handles DQ.DimensionError
4779
# We just need to make sure it can handle Unitful.DimensionError too

src/systems/unit_check.jl

Lines changed: 31 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,32 @@ function get_unit(op, args) # Fallback
8181
result = op(unit_args...)
8282
# For operations that return a unit directly, return it
8383
return screen_unit(result)
84-
catch
84+
catch e
85+
# If we get an ambiguous method error mixing DQ and Unitful,
86+
# try to handle common cases
87+
if e isa MethodError && length(unit_args) >= 2
88+
# Check if we have mixed DQ and Unitful units
89+
unit_types = _get_unittype.(unit_args)
90+
if Val(:DynamicQuantities) in unit_types && Val(:Unitful) in unit_types
91+
# For multiplication/division operations involving unitless and Unitful
92+
if op === (*)
93+
# If first argument is unitless (like -1), result should have the unit of the second
94+
if unit_args[1] == unitless && length(unit_args) == 2
95+
return screen_unit(unit_args[2])
96+
elseif length(unit_args) == 2 && unit_args[2] == unitless
97+
return screen_unit(unit_args[1])
98+
end
99+
elseif op === (/) && unit_args[1] == unitless
100+
# 1 / unitful_unit should give unitful_unit^-1
101+
try
102+
return screen_unit(inv(unit_args[2]))
103+
catch
104+
# Fall through to original error handling
105+
end
106+
end
107+
end
108+
end
109+
85110
try
86111
# Try with oneunit for numeric operations
87112
result = oneunit(op(unit_args...))
@@ -153,8 +178,11 @@ function get_unit(x::Symbolic)
153178
elseif ispow(x)
154179
pargs = arguments(x)
155180
base, expon = get_unit.(pargs)
156-
@assert oneunit(expon) == unitless
157-
if base == unitless
181+
# Check that exponent is dimensionless (works for both DQ and Unitful)
182+
if !equivalent(expon, unitless)
183+
throw(ValidationError("Power exponent must be dimensionless, got $expon"))
184+
end
185+
if equivalent(base, unitless)
158186
unitless
159187
else
160188
pargs[2] isa Number ? base^pargs[2] : (1 * base)^pargs[2]

test/units.jl

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -19,9 +19,9 @@ D = Differential(t)
1919
@test ModelingToolkit.get_unit(t) == u"ms"
2020
@test ModelingToolkit.get_unit(E) == u"kJ"
2121
@test ModelingToolkit.get_unit(τ) == u"ms"
22-
@test ModelingToolkit.get_unit(γ) == unitless
23-
@test ModelingToolkit.get_unit(0.5) == unitless
24-
@test ModelingToolkit.get_unit(ModelingToolkit.SciMLBase.NullParameters()) == unitless
22+
@test ModelingToolkit.equivalent(ModelingToolkit.get_unit(γ), unitless)
23+
@test ModelingToolkit.equivalent(ModelingToolkit.get_unit(0.5), unitless)
24+
@test ModelingToolkit.equivalent(ModelingToolkit.get_unit(ModelingToolkit.SciMLBase.NullParameters()), unitless)
2525

2626
# Prohibited unit types
2727
@parameters β [unit = u"°"] α [unit = u"°C"] γ [unit = 1u"s"]
@@ -34,14 +34,14 @@ D = Differential(t)
3434
@test ModelingToolkit.equivalent(ModelingToolkit.get_unit(D(E)), u"MW")
3535
@test ModelingToolkit.equivalent(ModelingToolkit.get_unit(E / τ), u"MW")
3636
@test ModelingToolkit.get_unit(2 * P) == u"MW"
37-
@test ModelingToolkit.get_unit(t / τ) == unitless
37+
@test ModelingToolkit.equivalent(ModelingToolkit.get_unit(t / τ), unitless)
3838
@test ModelingToolkit.equivalent(ModelingToolkit.get_unit(P - E / τ), u"MW")
3939
@test ModelingToolkit.equivalent(ModelingToolkit.get_unit(D(D(E))), u"MW/ms")
4040
@test ModelingToolkit.get_unit(ifelse(t > t, P, E / τ)) == u"MW"
41-
@test ModelingToolkit.get_unit(1.0^(t / τ)) == unitless
42-
@test ModelingToolkit.get_unit(exp(t / τ)) == unitless
43-
@test ModelingToolkit.get_unit(sin(t / τ)) == unitless
44-
@test ModelingToolkit.get_unit(sin(1 * u"rad")) == unitless
41+
@test ModelingToolkit.equivalent(ModelingToolkit.get_unit(1.0^(t / τ)), unitless)
42+
@test ModelingToolkit.equivalent(ModelingToolkit.get_unit(exp(t / τ)), unitless)
43+
@test ModelingToolkit.equivalent(ModelingToolkit.get_unit(sin(t / τ)), unitless)
44+
@test ModelingToolkit.equivalent(ModelingToolkit.get_unit(sin(1 * u"rad")), unitless)
4545
@test ModelingToolkit.get_unit(t^2) == u"ms^2"
4646

4747
eqs = [D(E) ~ P - E / τ
@@ -225,7 +225,7 @@ end
225225
@test ModelingToolkit.getdefault(sys.a) [0.01, 0.03]
226226

227227
@variables x(t)
228-
@test ModelingToolkit.get_unit(sin(x)) == unitless
228+
@test ModelingToolkit.equivalent(ModelingToolkit.get_unit(sin(x)), unitless)
229229

230230
@mtkmodel ExpressionParametersTest begin
231231
@parameters begin

0 commit comments

Comments
 (0)