From 40fae5bf889a19f9771edd3ab2668d1224105497 Mon Sep 17 00:00:00 2001 From: BDanaei Date: Fri, 24 Apr 2026 00:55:38 -0400 Subject: [PATCH 1/4] 1. Fix non-determinism and Heuristic 2 logic bug in carpanzano_tearing - sort(collect(active_eqs)) in find_single_solvable_eq! and carpanzano_tear_scc! to eliminate hash-order-dependent iteration over Set{Int} active_eqs and active_vars. - Fix Heuristic 2 in carpanzano_tear_scc!: max_incidence_cnt and min_solvable_cnt were never updated inside the loop body, causing alg_var to be overwritten every iteration (cnt > typemin(Int) is always true) and ending up as the last hash-order variable rather than the one with maximum incidence / minimum solvable count. Add the two missing tracker assignments. 2. Add unit test for carpanzano_tearing Heuristic 2 correctness Test calls carpanzano_tear_scc! directly with a minimal 3x3 bipartite graph where all edges are solvable (bypassing Heuristic 1) and v2 has incidence 3 (vs v1=2 and v3=2). Verifies that Heuristic 2 correctly selects v2 as the algebraic/torn variable. Before the fix, max_incidence_cnt was never updated inside the Heuristic 2 loop so the last variable in hash-iteration order always 'won', making the result non-deterministic and incorrect. --- src/carpanzano_tearing.jl | 11 +++++-- test/carpanzano_tearing.jl | 65 ++++++++++++++++++++++++++++++++++++++ test/runtests.jl | 1 + 3 files changed, 74 insertions(+), 3 deletions(-) create mode 100644 test/carpanzano_tearing.jl diff --git a/src/carpanzano_tearing.jl b/src/carpanzano_tearing.jl index 0b25dfb..122e08b 100644 --- a/src/carpanzano_tearing.jl +++ b/src/carpanzano_tearing.jl @@ -92,7 +92,8 @@ function find_single_solvable_eq!( ) where {F} (; graph, solvable_graph) = structure nbors = nbors_buffer - for ieq in active_eqs + # Sort active_eqs iteration for deterministic equation selection regardless of hash order + for ieq in sort(collect(active_eqs)) empty!(nbors) append!(nbors, Iterators.filter(in(active_vars), đť‘ neighbors(graph, ieq))) length(nbors) == 1 || continue @@ -161,7 +162,8 @@ function carpanzano_tear_scc!( # is the corresponding number of incident edges. empty!(enodes_with_min_incidence) min_incidence_cnt = typemax(Int) - for ieq in active_eqs + # Sort active_eqs for deterministic tie-breaking regardless of hash order + for ieq in sort(collect(active_eqs)) cnt = count(in(active_vars), đť‘ neighbors(graph, ieq)) cnt > min_incidence_cnt && continue if cnt == min_incidence_cnt @@ -206,11 +208,14 @@ function carpanzano_tear_scc!( alg_var = 0 max_incidence_cnt = typemin(Int) min_solvable_cnt = typemax(Int) - for ivar in active_vars + # Sort active_vars for deterministic selection; also fix missing max/min updates + for ivar in sort(collect(active_vars)) cnt = count(in(active_eqs), đť‘‘neighbors(graph, ivar)) solvable_cnt = count(in(active_eqs), đť‘‘neighbors(solvable_graph, ivar)) if iszero(alg_var) || cnt > max_incidence_cnt || cnt == max_incidence_cnt && solvable_cnt < min_solvable_cnt alg_var = ivar + max_incidence_cnt = cnt + min_solvable_cnt = solvable_cnt end end diff --git a/test/carpanzano_tearing.jl b/test/carpanzano_tearing.jl new file mode 100644 index 0000000..a3a9006 --- /dev/null +++ b/test/carpanzano_tearing.jl @@ -0,0 +1,65 @@ +# Tests for carpanzano_tearing.jl - determinism and Heuristic 2 correctness +# +# These tests call carpanzano_tear_scc! directly so they have no dependency on +# ModelingToolkit or any other heavy upstream package. + +using BipartiteGraphs +import Graphs: add_edge! + +# Minimal concrete SystemStructure used only in these tests. +struct TestSystemStructure <: StateSelection.SystemStructure + graph::BipartiteGraph{Int,Nothing} + solvable_graph::BipartiteGraph{Int,Nothing} + var_to_diff::DiffGraph + eq_to_diff::DiffGraph +end + +@testset "carpanzano_tearing" begin + @testset "Heuristic 2 picks variable with maximum incidence" begin + # Graph: 3 equations (source vertices), 3 variables (destination vertices). + # Every edge is also present in the solvable_graph (all variables solvable in + # all equations they appear in), so Heuristic 1 never finds a non-solvable + # variable and falls through to Heuristic 2. + # + # Incidence of each variable (number of equations it appears in): + # v1 (idx 1): eq1, eq2 => 2 + # v2 (idx 2): eq1, eq2, eq3 => 3 # maximum -> must be torn (algebraic) + # v3 (idx 3): eq1, eq3 => 2 + # + # Before the fix, max_incidence_cnt was never updated inside the Heuristic 2 + # loop, so every variable satisfied cnt > typemin(Int) and the last variable + # in hash-iteration order was chosen -- non-deterministic and wrong. + graph = BipartiteGraph(3, 3) + for (eq, var) in [(1,1),(1,2),(1,3),(2,1),(2,2),(3,2),(3,3)] + add_edge!(graph, eq, var) + end + solvable_graph = BipartiteGraph(3, 3) + for (eq, var) in [(1,1),(1,2),(1,3),(2,1),(2,2),(3,2),(3,3)] + add_edge!(solvable_graph, eq, var) + end + + structure = TestSystemStructure( + graph, solvable_graph, + DiffGraph(3), DiffGraph(3) + ) + alg = StateSelection.CarpanzanoTearing() + # complete() makes invview(var_eq_matching) available, which the algorithm uses + # internally to find the variable matched to a solved equation. + var_eq_matching = complete( + Matching{Union{Unassigned, SelectedState}}(3), 3 + ) + active_vars = Set{Int}([1, 2, 3]) + active_eqs = Set{Int}([1, 2, 3]) + + StateSelection.carpanzano_tear_scc!( + alg, structure, var_eq_matching, active_vars, active_eqs + ) + + # v2 has the highest incidence (3 equations) -- Heuristic 2 must choose it as + # the algebraic (torn) variable, leaving it unmatched. + @test var_eq_matching[2] === unassigned + # v1 and v3 must each be matched to some equation. + @test var_eq_matching[1] isa Int + @test var_eq_matching[3] isa Int + end +end diff --git a/test/runtests.jl b/test/runtests.jl index 399f56f..9162d14 100644 --- a/test/runtests.jl +++ b/test/runtests.jl @@ -4,6 +4,7 @@ using SparseArrays using Test include("bareiss.jl") +include("carpanzano_tearing.jl") @testset "`get_new_mm`" begin mm = SSel.CLIL.SparseMatrixCLIL( From ff3a9bb546dd7788a15863104ed9e42398446144 Mon Sep 17 00:00:00 2001 From: Behzad Danaei <52261510+behzadd1992@users.noreply.github.com> Date: Fri, 24 Apr 2026 01:33:00 -0400 Subject: [PATCH 2/4] Clean up comments in carpanzano_tearing.jl Clean up comments --- test/carpanzano_tearing.jl | 3 --- 1 file changed, 3 deletions(-) diff --git a/test/carpanzano_tearing.jl b/test/carpanzano_tearing.jl index a3a9006..d76d13e 100644 --- a/test/carpanzano_tearing.jl +++ b/test/carpanzano_tearing.jl @@ -1,7 +1,4 @@ # Tests for carpanzano_tearing.jl - determinism and Heuristic 2 correctness -# -# These tests call carpanzano_tear_scc! directly so they have no dependency on -# ModelingToolkit or any other heavy upstream package. using BipartiteGraphs import Graphs: add_edge! From 5c8c3cd7e25d5b9074a8eb02380b8f57ab7b8dbc Mon Sep 17 00:00:00 2001 From: Behzad Danaei <52261510+behzadd1992@users.noreply.github.com> Date: Fri, 24 Apr 2026 01:34:13 -0400 Subject: [PATCH 3/4] Refactor TestSystemStructure in carpanza_tearing.jl Remove comments and adjust struct definition. --- test/carpanzano_tearing.jl | 1 - 1 file changed, 1 deletion(-) diff --git a/test/carpanzano_tearing.jl b/test/carpanzano_tearing.jl index d76d13e..7899f13 100644 --- a/test/carpanzano_tearing.jl +++ b/test/carpanzano_tearing.jl @@ -3,7 +3,6 @@ using BipartiteGraphs import Graphs: add_edge! -# Minimal concrete SystemStructure used only in these tests. struct TestSystemStructure <: StateSelection.SystemStructure graph::BipartiteGraph{Int,Nothing} solvable_graph::BipartiteGraph{Int,Nothing} From 1b6c1262aafa532eb1ab06e839add5891ebd389c Mon Sep 17 00:00:00 2001 From: Behzad Danaei <52261510+behzadd1992@users.noreply.github.com> Date: Fri, 24 Apr 2026 10:28:34 -0400 Subject: [PATCH 4/4] refactor(tearing): use OrderedSet instead of sort() for deterministic SCC tearing --- src/carpanzano_tearing.jl | 17 +++++++---------- 1 file changed, 7 insertions(+), 10 deletions(-) diff --git a/src/carpanzano_tearing.jl b/src/carpanzano_tearing.jl index 122e08b..4c2101c 100644 --- a/src/carpanzano_tearing.jl +++ b/src/carpanzano_tearing.jl @@ -52,8 +52,8 @@ function (alg::CarpanzanoTearing)(structure::SystemStructure) full_var_eq_matching = copy(var_eq_matching) var_sccs = find_var_sccs(graph, var_eq_matching) - active_vars = Set{Int}() - active_eqs = Set{Int}() + active_vars = OrderedSet{Int}() + active_eqs = OrderedSet{Int}() for vars in var_sccs for var in vars # Identify variables and equations in this SCC @@ -87,13 +87,12 @@ In the context of the paper, `structure.graph` is the associated bipartite graph """ function find_single_solvable_eq!( structure::SystemStructure, var_eq_matching::MatchingT, - active_vars::Set{Int}, active_eqs::Set{Int}, condition::F = _ -> true; + active_vars::AbstractSet{Int}, active_eqs::AbstractSet{Int}, condition::F = _ -> true; nbors_buffer::Vector{Int} = Int[] ) where {F} (; graph, solvable_graph) = structure nbors = nbors_buffer - # Sort active_eqs iteration for deterministic equation selection regardless of hash order - for ieq in sort(collect(active_eqs)) + for ieq in active_eqs empty!(nbors) append!(nbors, Iterators.filter(in(active_vars), đť‘ neighbors(graph, ieq))) length(nbors) == 1 || continue @@ -117,7 +116,7 @@ all of `active_vars` to `unassigned`, and will be modified to match solvable var """ function carpanzano_tear_scc!( alg::CarpanzanoTearing, structure::SystemStructure, var_eq_matching::MatchingT, - active_vars::Set{Int}, active_eqs::Set{Int} + active_vars::AbstractSet{Int}, active_eqs::AbstractSet{Int} ) # TODO: This is an implementation of algorithm A1 in the paper. Find an efficient # way to implement algorithm A2 and analyze the benefits. @@ -162,8 +161,7 @@ function carpanzano_tear_scc!( # is the corresponding number of incident edges. empty!(enodes_with_min_incidence) min_incidence_cnt = typemax(Int) - # Sort active_eqs for deterministic tie-breaking regardless of hash order - for ieq in sort(collect(active_eqs)) + for ieq in active_eqs cnt = count(in(active_vars), đť‘ neighbors(graph, ieq)) cnt > min_incidence_cnt && continue if cnt == min_incidence_cnt @@ -208,8 +206,7 @@ function carpanzano_tear_scc!( alg_var = 0 max_incidence_cnt = typemin(Int) min_solvable_cnt = typemax(Int) - # Sort active_vars for deterministic selection; also fix missing max/min updates - for ivar in sort(collect(active_vars)) + for ivar in active_vars cnt = count(in(active_eqs), đť‘‘neighbors(graph, ivar)) solvable_cnt = count(in(active_eqs), đť‘‘neighbors(solvable_graph, ivar)) if iszero(alg_var) || cnt > max_incidence_cnt || cnt == max_incidence_cnt && solvable_cnt < min_solvable_cnt