Merge pull request #3683 from AayushSabharwal/as/rm-isequal-hash

refactor: remove old `==` and `hash` implementation for `System`

Author: ChrisRackauckas

src/systems/system.jl

@@ -784,97 +784,6 @@ function ignored_connections_equal(sys1::System, sys2::System)
     return _eq_unordered(ic1[1], ic2[1]) && _eq_unordered(ic1[2], ic2[2])
 end
 
-function Base.:(==)(sys1::System, sys2::System)
-    sys1 === sys2 && return true
-    iv1 = get_iv(sys1)
-    iv2 = get_iv(sys2)
-    isequal(iv1, iv2) &&
-        isequal(nameof(sys1), nameof(sys2)) &&
-        _eq_unordered(get_eqs(sys1), get_eqs(sys2)) &&
-        noise_equations_equal(sys1, sys2) &&
-        _eq_unordered(get_jumps(sys1), get_jumps(sys2)) &&
-        _eq_unordered(get_constraints(sys1), get_constraints(sys2)) &&
-        _eq_unordered(get_costs(sys1), get_costs(sys2)) &&
-        isequal(get_consolidate(sys1), get_consolidate(sys2)) &&
-        _eq_unordered(get_unknowns(sys1), get_unknowns(sys2)) &&
-        _eq_unordered(get_ps(sys1), get_ps(sys2)) &&
-        _eq_unordered(get_brownians(sys1), get_brownians(sys2)) &&
-        _eq_unordered(get_observed(sys1), get_observed(sys2)) &&
-        _eq_unordered(get_parameter_dependencies(sys1), get_parameter_dependencies(sys2)) &&
-        isequal(get_description(sys1), get_description(sys2)) &&
-        isequal(get_defaults(sys1), get_defaults(sys2)) &&
-        isequal(get_guesses(sys1), get_guesses(sys2)) &&
-        _eq_unordered(get_initialization_eqs(sys1), get_initialization_eqs(sys2)) &&
-        _eq_unordered(get_continuous_events(sys1), get_continuous_events(sys2)) &&
-        _eq_unordered(get_discrete_events(sys1), get_discrete_events(sys2)) &&
-        isequal(get_connector_type(sys1), get_connector_type(sys2)) &&
-        isequal(get_assertions(sys1), get_assertions(sys2)) &&
-        isequal(get_metadata(sys1), get_metadata(sys2)) &&
-        isequal(get_gui_metadata(sys1), get_gui_metadata(sys2)) &&
-        get_is_dde(sys1) == get_is_dde(sys2) &&
-        _eq_unordered(get_tstops(sys1), get_tstops(sys2)) &&
-        # not comparing tearing states because checking if they're equal up to ordering
-        # is difficult
-        getfield(sys1, :namespacing) == getfield(sys2, :namespacing) &&
-        getfield(sys1, :complete) == getfield(sys2, :complete) &&
-        ignored_connections_equal(sys1, sys2) &&
-        get_parent(sys1) == get_parent(sys2) &&
-        get_isscheduled(sys1) == get_isscheduled(sys2) &&
-        all(s1 == s2 for (s1, s2) in zip(get_systems(sys1), get_systems(sys2)))
-end
-
-function Base.hash(sys::System, h::UInt)
-    h = hash(nameof(sys), h)
-    h = hash(get_iv(sys), h)
-    # be considerate of things compared using `_eq_unordered` in `==`
-    eqs = get_eqs(sys)
-    eq_sortperm = sortperm(eqs; by = string)
-    h = hash(@view(eqs[eq_sortperm]), h)
-    neqs = get_noise_eqs(sys)
-    if neqs === nothing
-        h = hash(nothing, h)
-    elseif neqs isa Vector
-        h = hash(@view(neqs[eq_sortperm]), h)
-    else
-        h = hash(@view(neqs[eq_sortperm, :]), h)
-    end
-    h = hash(Set(get_jumps(sys)), h)
-    h = hash(Set(get_constraints(sys)), h)
-    h = hash(Set(get_costs(sys)), h)
-    h = hash(get_consolidate(sys), h)
-    h = hash(Set(get_unknowns(sys)), h)
-    h = hash(Set(get_ps(sys)), h)
-    h = hash(Set(get_brownians(sys)), h)
-    h = hash(Set(get_observed(sys)), h)
-    h = hash(Set(get_parameter_dependencies(sys)), h)
-    h = hash(get_description(sys), h)
-    h = hash(get_defaults(sys), h)
-    h = hash(get_guesses(sys), h)
-    h = hash(Set(get_initialization_eqs(sys)), h)
-    h = hash(Set(get_continuous_events(sys)), h)
-    h = hash(Set(get_discrete_events(sys)), h)
-    h = hash(get_connector_type(sys), h)
-    h = hash(get_assertions(sys), h)
-    h = hash(get_metadata(sys), h)
-    h = hash(get_gui_metadata(sys), h)
-    h = hash(get_is_dde(sys), h)
-    h = hash(Set(get_tstops(sys)), h)
-    h = hash(Set(getfield(sys, :namespacing)), h)
-    h = hash(Set(getfield(sys, :complete)), h)
-    ics = get_ignored_connections(sys)
-    if ics === nothing
-        h = hash(ics, h)
-    else
-        h = hash(Set(ics[1]), hash(Set(ics[2]), h), h)
-    end
-    h = hash(get_parent(sys), h)
-    h = hash(get_isscheduled(sys), h)
-    for s in get_systems(sys)
-        h = hash(s, h)
-    end
-    return h
-end
-
 """
     $(TYPEDSIGNATURES)
 

test/accessor_functions.jl

@@ -151,17 +151,11 @@ let
     # Checks `continuous_events_toplevel` and  `discrete_events_toplevel` (straightforward
     # as I stored the same single event in all systems). Don't check for non-toplevel cases as
     # technically not needed for these tests and name spacing the events is a mess.
-    bot_cev = ModelingToolkit.SymbolicContinuousCallback(
-        cevs[1], alg_eqs = [O ~ (d + p_bot) * X_bot + Y])
-    mid_dev = ModelingToolkit.SymbolicDiscreteCallback(
-        devs[1], alg_eqs = [O ~ (d + p_mid1) * X_mid1 + Y])
     @test all_sets_equal(
-        continuous_events_toplevel.([sys_bot, sys_bot_comp, sys_bot_ss])...,
-        [bot_cev])
+        continuous_events_toplevel.([sys_bot, sys_bot_comp, sys_bot_ss])...)
     @test all_sets_equal(
         discrete_events_toplevel.(
-            [sys_mid1, sys_mid1_comp, sys_mid1_ss])...,
-        [mid_dev])
+        [sys_mid1, sys_mid1_comp, sys_mid1_ss])...)
     @test all(sym_issubset(
                   continuous_events_toplevel(sys), get_continuous_events(sys))
     for sys in [sys_bot, sys_mid2, sys_mid1, sys_top])

test/analysis_points.jl

@@ -22,7 +22,7 @@ using Symbolics: NAMESPACE_SEPARATOR
     sys_normal = System(eqs, t, systems = [P, C], name = :hej)
     sys_normal2 = @test_nowarn expand_connections(sys_normal)
 
-    @test isequal(sys_ap2, sys_normal2)
+    @test issetequal(equations(sys_ap2), equations(sys_normal2))
 end
 
 @testset "Inverse causality throws a warning" begin

test/nonlinearsystem.jl

@@ -26,7 +26,10 @@ eqs = [0 ~ σ * (y - x) * h,
     0 ~ x * (ρ - z) - y,
     0 ~ x * y - β * z]
 @named ns = System(eqs, [x, y, z], [σ, ρ, β, h], defaults = Dict(x => 2))
-@test eval(toexpr(ns)) == ns
+ns2 = eval(toexpr(ns))
+@test issetequal(equations(ns), equations(ns2))
+@test issetequal(unknowns(ns), unknowns(ns2))
+@test issetequal(parameters(ns), parameters(ns2))
 test_nlsys_inference("standard", ns, (x, y, z), (σ, ρ, β, h))
 @test begin
     f = generate_rhs(ns, expression = Val{false})[2]

test/odesystem.jl

@@ -35,8 +35,10 @@ ssort(eqs) = sort(eqs, by = string)
 @named des[1:3] = System(eqs, t)
 @test length(unique(x -> ModelingToolkit.get_tag(x), des)) == 1
 
-@test eval(toexpr(de)) == de
-@test hash(deepcopy(de)) == hash(de)
+de2 = eval(toexpr(de))
+@test issetequal(equations(de2), eqs)
+@test issetequal(unknowns(de2), unknowns(de))
+@test issetequal(parameters(de2), parameters(de))
 
 function test_diffeq_inference(name, sys, iv, dvs, ps)
     @testset "System construction: $name" begin
@@ -710,7 +712,9 @@ let
     s1′ = sys1(; name = :s1)
     @named s2 = sys2()
     @unpack s1 = s2
-    @test isequal(s1, s1′)
+    @test isequal(unknowns(s1), unknowns(s1′))
+    @test isequal(parameters(s1), parameters(s1′))
+    @test isequal(equations(s1), equations(s1′))
 
     defs = Dict(s1.dx => 0.0, D(s1.x) => s1.x, s1.x => 0.0)
     @test isequal(ModelingToolkit.defaults(s2), defs)
@@ -1427,33 +1431,6 @@ end
     @test_nowarn @named osys = System(eqs, t)
 end
 
-# Test `isequal`
-@testset "`isequal`" begin
-    @variables X(t)
-    @parameters p d(t)
-    eq = D(X) ~ p - d * X
-
-    osys1 = complete(System([eq], t; name = :osys))
-    osys2 = complete(System([eq], t; name = :osys))
-    @test osys1 == osys2 # true
-
-    continuous_events = [[X ~ 1.0] => [X ~ Pre(X) + 5.0]]
-    discrete_events = [SymbolicDiscreteCallback(
-        5.0 => [d ~ d / 2.0], discrete_parameters = [d])]
-
-    osys1 = complete(System([eq], t; name = :osys, continuous_events))
-    osys2 = complete(System([eq], t; name = :osys))
-    @test osys1 !== osys2
-
-    osys1 = complete(System([eq], t; name = :osys, discrete_events))
-    osys2 = complete(System([eq], t; name = :osys))
-    @test osys1 !== osys2
-
-    osys1 = complete(System([eq], t; name = :osys, continuous_events))
-    osys2 = complete(System([eq], t; name = :osys, discrete_events))
-    @test osys1 !== osys2
-end
-
 @testset "Constraint system construction" begin
     @variables x(..) y(..) z(..)
     @parameters a b c d e

test/sdesystem.jl

@@ -612,9 +612,8 @@ diffusion_eqs = [s*x 0
 
 sys2 = SDESystem(drift_eqs, diffusion_eqs, tt, sts, ps, name = :sys1)
 sys2 = complete(sys2)
-@set! sys1.parent = nothing
-@set! sys2.parent = nothing
-@test sys1 == sys2
+
+@test issetequal(ModelingToolkit.get_noise_eqs(sys1), ModelingToolkit.get_noise_eqs(sys2))
 
 prob = SDEProblem(sys1, [sts .=> [1.0, 0.0, 0.0]; ps .=> [10.0, 26.0]],
     (0.0, 100.0))
@@ -927,31 +926,6 @@ end
     end
 end
 
-@testset "SDESystem Equality with events" begin
-    @variables X(t)
-    @parameters p d
-    @brownian a
-    seq = D(X) ~ p - d * X + a
-    @mtkcompile ssys1 = System([seq], t; name = :ssys)
-    @mtkcompile ssys2 = System([seq], t; name = :ssys)
-    @test ssys1 == ssys2 # true
-
-    continuous_events = [[X ~ 1.0] => [X ~ Pre(X) + 5.0]]
-    discrete_events = [5.0 => [d ~ Pre(d) / 2.0]]
-
-    @mtkcompile ssys1 = System([seq], t; name = :ssys, continuous_events)
-    @mtkcompile ssys2 = System([seq], t; name = :ssys)
-    @test ssys1 !== ssys2
-
-    @mtkcompile ssys1 = System([seq], t; name = :ssys, discrete_events)
-    @mtkcompile ssys2 = System([seq], t; name = :ssys)
-    @test ssys1 !== ssys2
-
-    @mtkcompile ssys1 = System([seq], t; name = :ssys, continuous_events)
-    @mtkcompile ssys2 = System([seq], t; name = :ssys, discrete_events)
-    @test ssys1 !== ssys2
-end
-
 @testset "Error when constructing SDEProblem without `mtkcompile`" begin
     @parameters σ ρ β
     @variables x(tt) y(tt) z(tt)

test/serialization.jl

@@ -27,7 +27,10 @@ write(io, expand_connections(rc_model))
 str = String(take!(io))
 
 sys = include_string(@__MODULE__, str)
-@test sys == expand_connections(rc_model) # this actually kind of works, but the variables would have different identities.
+rc2 = expand_connections(rc_model)
+@test issetequal(equations(sys), equations(rc2))
+@test issetequal(unknowns(sys), unknowns(rc2))
+@test issetequal(parameters(sys), parameters(rc2))
 
 # check answer
 ss = mtkcompile(rc_model)

test/symbolic_events.jl

@@ -310,8 +310,9 @@ end
         [D(x) ~ v
          D(v) ~ -9.8], t, continuous_events = root_eqs => affect)
 
-    @test only(continuous_events(ball)) ==
-          SymbolicContinuousCallback(Equation[x ~ 0], Equation[v ~ -Pre(v)])
+    cev = only(continuous_events(ball))
+    @test isequal(only(equations(cev)), x ~ 0)
+    @test isequal(only(observed(cev.affect.system)), v ~ -Pre(v))
     ball = mtkcompile(ball)
 
     @test length(ModelingToolkit.continuous_events(ball)) == 1
@@ -343,10 +344,11 @@ end
 
     cb = get_callback(prob)
     @test cb isa ModelingToolkit.DiffEqCallbacks.VectorContinuousCallback
-    @test getfield(ball, :continuous_events)[1] ==
-          SymbolicContinuousCallback(Equation[x ~ 0], Equation[vx ~ -Pre(vx)])
-    @test getfield(ball, :continuous_events)[2] ==
-          SymbolicContinuousCallback(Equation[y ~ -1.5, y ~ 1.5], Equation[vy ~ -Pre(vy)])
+    _cevs = getfield(ball, :continuous_events)
+    @test isequal(only(equations(_cevs[1])), x ~ 0)
+    @test isequal(only(observed(_cevs[1].affect.system)), vx ~ -Pre(vx))
+    @test issetequal(equations(_cevs[2]), [y ~ -1.5, y ~ 1.5])
+    @test isequal(only(observed(_cevs[2].affect.system)), vy ~ -Pre(vy))
     cond = cb.condition
     out = [0.0, 0.0, 0.0]
     p0 = 0.0