From e9239baeae36dfc17768ea1b26fe683f7a1b7fe8 Mon Sep 17 00:00:00 2001
From: ChrisRackauckas <accounts@chrisrackauckas.com>
Date: Wed, 23 Jul 2025 00:39:00 -0400
Subject: [PATCH] Fix unit validation for dimensionally compatible quantities
 (fixes #3787)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

This commit fixes the unit validation system to properly handle dimensionally
compatible quantities with different scales (e.g., MW vs kW, V vs mV).

The issue was that the `equivalent()` function used exact equality (`isequal`)
instead of dimensional compatibility checking. This caused validation to fail
for physically meaningful equations like `D(E) ~ P - E/τ` where E is in kJ,
P is in MW, and τ is in ms, even though kJ/ms = kW and MW and kW are
dimensionally compatible (both power units).

Changes:
- Updated `equivalent()` function to check dimensional compatibility using
  `DQ.dimension()` for DynamicQuantities instead of exact equality
- Added `equivalent_strict()` function for connection validation that maintains
  exact equality requirements for electrical connector safety
- Updated connection validation to use strict equivalence to prevent connecting
  V and mV connectors (different voltage scales)
- Added comprehensive tests including the original bug case and edge cases

The fix allows the example from issue #3787 to validate correctly while
maintaining safety for electrical connections.

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>
---
 src/systems/unit_check.jl | 24 ++++++++++++++++++++++--
 test/dq_units.jl          | 22 ++++++++++++++++++++++
 2 files changed, 44 insertions(+), 2 deletions(-)

diff --git a/src/systems/unit_check.jl b/src/systems/unit_check.jl
index acf7451065..1f030a998e 100644
--- a/src/systems/unit_check.jl
+++ b/src/systems/unit_check.jl
@@ -104,7 +104,27 @@ function get_unit(op::Integral, args)
     return get_unit(args[1]) * unit
 end
 
-equivalent(x, y) = isequal(x, y)
+function equivalent(x, y)
+    # For DynamicQuantities, check dimensional compatibility rather than exact equality
+    if x isa DQ.AbstractQuantity && y isa DQ.AbstractQuantity
+        return DQ.dimension(x) == DQ.dimension(y)
+    elseif !(x isa DQ.AbstractQuantity) && !(y isa DQ.AbstractQuantity)
+        # Both are not quantities, use exact equality
+        return isequal(x, y)
+    else
+        # One is quantity, one is not - check if quantity is dimensionless
+        q = x isa DQ.AbstractQuantity ? x : y
+        non_q = x isa DQ.AbstractQuantity ? y : x
+        # Only consider equivalent if the quantity is dimensionless and other is zero/number
+        return DQ.dimension(q) == DQ.dimension(DQ.Quantity(1.0)) && (isequal(non_q, 0) || non_q isa Number)
+    end
+end
+
+# For connections, we may want stricter equivalence (same scale for safety)
+function equivalent_strict(x, y)
+    # Use exact equality for connections to ensure same scales
+    return isequal(x, y)
+end
 function get_unit(op::Conditional, args)
     terms = get_unit.(args)
     terms[1] == unitless ||
@@ -234,7 +254,7 @@ function _validate(conn::Connection; info::String = "")
             else
                 aunit = safe_get_unit(x, info * string(nameof(sys)) * "#$i")
                 bunit = safe_get_unit(sst[j], info * string(nameof(s)) * "#$j")
-                if !equivalent(aunit, bunit)
+                if !equivalent_strict(aunit, bunit)
                     valid = false
                     str = "$info: connected system unknowns $x ($aunit) and $(sst[j]) ($bunit) have mismatched units."
                     if oneunit(aunit) == oneunit(bunit)
diff --git a/test/dq_units.jl b/test/dq_units.jl
index ea1103db57..3e7a3a3558 100644
--- a/test/dq_units.jl
+++ b/test/dq_units.jl
@@ -283,3 +283,25 @@ end
 @test ModelingToolkit.get_unit.(filter(x -> occursin("ˍt", string(x)), unknowns(pend))) ==
       [u"m/s", u"m/s"]
 end
+
+# Test for issue #3787: Validation of units with non-SI units should work
+@testset "Issue #3787: Non-SI unit validation" begin
+    @parameters τ_ms [unit = u"ms"]
+    @variables t_ms [unit = u"ms"] E_kJ(t_ms) [unit = u"kJ"] P_MW(t_ms) [unit = u"MW"]
+    D_ms = Differential(t_ms)
+    
+    # This should pass: kJ/ms = kW, and MW and kW are dimensionally compatible (both power)
+    eqs_3787 = [D_ms(E_kJ) ~ P_MW - E_kJ / τ_ms,
+                0 ~ P_MW]
+    @test MT.validate(eqs_3787) == true
+    
+    # Additional test: compatible units of different scales
+    @variables x_m [unit = u"m"] y_km [unit = u"km"]
+    eq_scales = [x_m ~ y_km]
+    @test MT.validate(eq_scales) == true
+    
+    # Test that incompatible dimensions still fail
+    @variables a_length [unit = u"m"] b_time [unit = u"s"]
+    eq_incompatible = [a_length ~ b_time]
+    @test MT.validate(eq_incompatible) == false
+end