Fix non-deterministic behavior and incorrect variable selection in Carpanzano tearing algorithm

[behzadd1992]

Summary

This PR fixes two related issues in carpanzano_tearing.jl that caused non-deterministic behavior and incorrect variable selection in the Carpanzano tearing algorithm.

---

Bugs Fixed

1. Heuristic 2 — tracker variables never updated (logic bug)

In carpanzano_tear_scc!, Heuristic 2 selects the algebraic (torn) variable with maximum incidence. However, the tracking variables max_incidence_cnt and min_solvable_cnt were never updated inside the loop:

# Before (buggy)
if iszero(alg_var) || cnt > max_incidence_cnt || ...
    alg_var = ivar
    # max_incidence_cnt = cnt   ← missing!
    # min_solvable_cnt = ...    ← missing!
end

Because max_incidence_cnt remained at typemin(Int), every variable satisfied the condition. As a result, the last iterated variable was always selected, ignoring incidence counts entirely.

This PR fixes the issue by correctly updating both tracking variables within the loop.

---

2. Non-determinism from hash-order iteration

Several loops iterated over Set{Int} collections (active_eqs, active_vars), whose iteration order depends on hash values and may vary across Julia versions and runs.

This resulted in non-reproducible tearing decisions, since the heuristics iterate over these sets and pick the "first" element satisfying a condition (single solvable eq, min-incidence eq, max-incidence var).

This PR fixes the issue at the data-structure level by switching active_vars and active_eqs from Set{Int} to OrderedSet{Int}. OrderedSet iterates in insertion order, which is itself deterministic here because the sets are populated by walking find_var_sccs(...) over a bipartite graph whose adjacency lists are Vector{Int} (not hash-backed) and a Vector-backed Matching — so no upstream changes are required.

---

Changes

• (alg::CarpanzanoTearing)(structure::SystemStructure):
  • active_vars = Set{Int}() → active_vars = OrderedSet{Int}()
  • active_eqs = Set{Int}() → active_eqs = OrderedSet{Int}()
• find_single_solvable_eq!: relax parameter types from Set{Int} to AbstractSet{Int} so the helper accepts both Set and OrderedSet (required because OrderedSet{Int} is a sibling of Set{Int}, not a subtype — both are <: AbstractSet{Int}).
• carpanzano_tear_scc! (Heuristic 2):
  • Fix missing updates to max_incidence_cnt and min_solvable_cnt inside the selection loop.
• Added using OrderedCollections: OrderedSet (or equivalent import) where needed.

No public API changes.

---

Tests Added

• Added a new test (carpanzano_tearing.jl) that directly calls carpanzano_tear_scc!
• Uses a minimal 3×3 fully solvable bipartite graph:
  • v2 has incidence 3
  • v1 and v3 have incidence 2
• Verifies that Heuristic 2 correctly selects v2 as the algebraic variable

Result:

• Fails on the original implementation
• Passes with this fix

---

Impact

• Ensures correct heuristic behavior
• Guarantees deterministic and reproducible results without per-iteration sorting overhead
• Determinism is enforced by the container type, reducing the risk of regressions in future call sites
• Improves reliability across Julia versions and execution environments

[AayushSabharwal]

Thank you for spotting the error and finding the fix. The review comments are for some performance concerns, but otherwise this looks great.

[AayushSabharwal]

This would also be resolved if active_eqs is made an OrderedSet{Int} instead of Set{Int}, since then the equations are always iterated over in insertion order. This approach avoids two allocations from collect and sort.

[behzadd1992]

Thanks for reviewing, Aayush!
I considered that direction as well, but in this code path active_eqs / active_vars are populated from SCC data that itself comes from Set/Dict-backed structures, so their insertion order is already hash-dependent. If we simply switch to OrderedSet{Int}, we'd just be preserving that hash-dependent insertion order and would still have the same non-determinism problem.

To make OrderedSet a drop-in replacement here, we'd also need to ensure those sets are constructed in a deterministic way (e.g. inserting elements in sorted order during SCC decomposition), which is a larger refactor touching the callers and the SCC building logic.

Given that, I went with the explicit sort(collect(...)) as the minimal, local fix that:

• removes the hash-order dependence unconditionally, and
• makes the tie-breaking behavior (lowest index first) explicit and obvious at the call site.

An OrderedSet-based approach would be a worthwhile follow-up optimization, but only after auditing and sorting all insertion sites in the SCC decomposition.

[behzadd1992]

I went back and traced the SCC construction more carefully, and I think your suggestion is actually safe to apply directly. I commited the suggested changes.

[AayushSabharwal]

Yep, we sort SCCs

[AayushSabharwal]

This can also be removed if active_eqs = OrderedSet{Int}()

[AayushSabharwal]

The same applies for active_vars

[AayushSabharwal]

Will merge after analyzing the new (1, InterfaceI) failure

[AayushSabharwal]

Thanks for the fix!