From 43a7936abd28ef51aa16be3f77569449d43ba906 Mon Sep 17 00:00:00 2001 From: Aayush Sabharwal Date: Wed, 17 Jun 2026 17:44:19 +0530 Subject: [PATCH 1/2] refactor: positive priority variables are no longer "sticky" in `eliminate_perfect_aliases!` Co-authored-by: Claude --- src/systems/alias_elimination.jl | 31 +++++++++++++++++++------------ 1 file changed, 19 insertions(+), 12 deletions(-) diff --git a/src/systems/alias_elimination.jl b/src/systems/alias_elimination.jl index d3c9316fca..b1ee9fe5fb 100644 --- a/src/systems/alias_elimination.jl +++ b/src/systems/alias_elimination.jl @@ -103,9 +103,11 @@ end Pick the target variable for an alias group. Irreducible variables must remain as unknowns, so one of them is chosen as the target when the group contains any. Otherwise -the variable with the highest `state_priority` wins. When priorities are tied, prefer -the variable appearing in the most equations: visualization-only variables appear in a -single alias equation, while physics variables appear in many dynamics equations. +the variable with the highest `state_priority` wins; all other group members are +eliminated in favour of the target. When positive priorities are tied, emit a warning and prefer the +variable appearing in the most equations (visualization-only variables appear in a single alias +equation, while physics variables appear in many dynamics equations); if that is also +tied, one of the tied variables is chosen arbitrarily. """ function pick_alias_target( fullvars::Vector{SymbolicT}, group_vars::Vector{Int}, state_priorities, irreducibles::AtomicSetT, @@ -118,9 +120,14 @@ function pick_alias_target( irr_idx === nothing || return group_vars[irr_idx] max_priority = maximum(Base.Fix1(getindex, state_priorities), group_vars) candidates = filter(v -> state_priorities[v] == max_priority, group_vars) - if graph !== nothing && length(candidates) > 1 - _, target_idx = findmax(v -> length(𝑑neighbors(graph, v)), candidates) - return candidates[target_idx] + if length(candidates) > 1 && max_priority > 0 + tied_names = getindex.(Ref(fullvars), candidates) + @warn "Multiple variables in an alias group share the highest state_priority \ +($max_priority); choosing alias target by equation count. Tied variables: $tied_names" + if graph !== nothing + max_degree = maximum(v -> length(𝑑neighbors(graph, v)), candidates) + candidates = filter(v -> length(𝑑neighbors(graph, v)) == max_degree, candidates) + end end return candidates[1] end @@ -203,7 +210,6 @@ function find_perfect_aliases!( group_target = Dict{Int, Int}() sizehint!(group_target, length(members)) - is_sticky = (v) -> contains_possibly_indexed_element(irreducibles, fullvars[v]) || state_priorities[v] > 0 is_irreducible_v = (v) -> contains_possibly_indexed_element(irreducibles, fullvars[v]) # Symbolic zero used as substitution target for variables in conflict groups. @@ -250,9 +256,10 @@ function find_perfect_aliases!( group_target[root] = target target_p = parity[target] for v in group_vars - # In consistent groups sticky vars (irreducible OR priority>0) other - # than the target stay as unknowns. - if is_sticky(v) || v == target + # In consistent groups, irreducible vars other than the target stay as unknowns. + # All other non-target vars (including those with positive state_priority) are + # eliminated in favour of the highest-priority target. + if is_irreducible_v(v) || v == target state.always_present[v] = true continue end @@ -318,8 +325,8 @@ function find_perfect_aliases!( end else target = group_target[parent[v1]] - c1 = is_sticky(v1) ? v1 : target - c2 = is_sticky(v2) ? v2 : target + c1 = is_irreducible_v(v1) ? v1 : target + c2 = is_irreducible_v(v2) ? v2 : target c1 == c2 && push!(eqs_to_rm, ieq) end end From 65f04dbf3ff193606d4cd1135bc0cd478531301c Mon Sep 17 00:00:00 2001 From: Aayush Sabharwal Date: Wed, 17 Jun 2026 17:47:41 +0530 Subject: [PATCH 2/2] test: test changes to `eliminate_perfect_aliases!` Co-authored-by: Claude --- test/reduction.jl | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/test/reduction.jl b/test/reduction.jl index 7cc9b0e6f3..f2e73bb65f 100644 --- a/test/reduction.jl +++ b/test/reduction.jl @@ -359,6 +359,34 @@ ss = mtkcompile(sys) @test isequal(only(unknowns(sys)), y) end +@testset "positive-priority variable is eliminated when target has higher priority" begin + # Both x and y have positive state_priority, but y's is higher. + # Old behaviour: both sticky → neither eliminated. + # New behaviour: x (lower priority) is eliminated as observed in favour of y. + @variables x(t) y(t) + @named sys = System([D(y) ~ -y, x ~ y], t; state_priorities = [x => 1, y => 2]) + state = TearingState(sys) + ModelingToolkit.eliminate_perfect_aliases!(state) + @test any(eq -> isequal(eq.lhs, unwrap(x)), state.additional_observed) + @test any(isequal(unwrap(y)), state.fullvars) + @test !any(isequal(unwrap(x)), state.fullvars) +end + +@testset "warning is emitted when state_priority is tied across alias group" begin + # x appears in two equations (D(x) ~ -x and x ~ y), y in one (x ~ y), + # so the equation-count heuristic picks x as target — but the priority tie + # (both at 5) must still emit a warning regardless of how the tie is broken. + @variables x(t) y(t) + @named sys = System([D(x) ~ -x, x ~ y], t; state_priorities = [x => 5, y => 5]) + state = TearingState(sys) + @test_logs (:warn, r"state_priority") match_mode=:any ModelingToolkit.eliminate_perfect_aliases!(state) + # Exactly one of the two variables is eliminated as observed. + n_elim = count( + eq -> isequal(eq.lhs, unwrap(x)) || isequal(eq.lhs, unwrap(y)), + state.additional_observed) + @test n_elim == 1 +end + @testset "Perfect aliases do not eliminate irreducible variables" begin @variables x(t) y(t) @variables e(t) [irreducible = true]