fix: handle disconnected components in tropical TN contraction

[ChanceSiyuan]

Summary

• Fix bug where omeco.optimize_code only processed one connected component of the factor graph
• Add connected component detection using union-find algorithm
• Update analyze_tropical_threshold.py to use pymatching graph structure
• Add comprehensive tests for the fix

Problem

The tropical TN decoder was producing incorrect results because disconnected factors (like single-variable prior factors) were being dropped from the contraction tree. In a 78-variable, 102-factor problem, only 31 factors were being contracted.

Solution

1. Added _find_connected_components() to detect all connected components
2. Modified get_omeco_tree() to contract each component separately and combine results
3. Now all factors are included in the contraction tree

Verification

After the fix:

• Tropical TN matches MWPM on 100% of test samples
• All 16 new tests pass
• Existing tests unchanged

Fixes #68

Test plan

• Run pytest tropical_in_new/tests/test_connected_components.py -v
• Run pytest tropical_in_new/tests/test_tropical_mwpm_match.py -v
• Verify Tropical TN matches MWPM on surface code samples

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 96.84%. Comparing base (9bdd0f7) to head (166e6b5).
⚠️ Report is 2 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main      #70      +/-   ##
==========================================
+ Coverage   95.18%   96.84%   +1.65%     
==========================================
  Files          10       10              
  Lines         747      728      -19     
==========================================
- Hits          711      705       -6     
+ Misses         36       23      -13     

Flag	Coverage Δ
unittests	96.84% <100.00%> (+1.65%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[Copilot]

Pull request overview

This PR fixes a bug in the tropical tensor network decoder where omeco.optimize_code was only processing one connected component of the factor graph, causing disconnected factors (like single-variable prior factors) to be dropped from the contraction tree. The fix implements connected component detection using a union-find algorithm and ensures all factors are included in the contraction.

Changes:

• Added _find_connected_components() function using union-find algorithm to detect all connected components in the factor graph
• Modified get_omeco_tree() to contract each component separately and combine results into a single tree
• Updated analysis scripts to use pymatching for comparison and validation
• Added comprehensive test suite covering various connected/disconnected component scenarios

Reviewed changes

Copilot reviewed 5 out of 6 changed files in this pull request and generated 8 comments.

Show a summary per file

File	Description
tropical_in_new/src/contraction.py	Core fix: added connected component detection and multi-component tree combination logic
tropical_in_new/tests/test_connected_components.py	Unit tests for connected component detection and tree construction
tropical_in_new/tests/test_tropical_mwpm_match.py	Integration tests comparing Tropical TN with MWPM decoder
scripts/analyze_tropical_threshold.py	Threshold analysis script using the fixed decoder
scripts/test_tropical_fix.py	Quick verification script for the fix
outputs/tropical_threshold_plot.png	Generated visualization output

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Inconsistent handling of empty detector factors between test_tropical_mwpm_match.py and analyze_tropical_threshold.py.

In test_tropical_mwpm_match.py (lines 74-80), when a detector has no connected errors (empty variable list), the factor value depends on the syndrome:

• If syndrome[d] == 0: assigns "1.0"
• If syndrome[d] != 0: assigns "1e-30"

However, in analyze_tropical_threshold.py (lines 154-157), the implementation always assigns "1.0" regardless of the syndrome value when a detector has no connected errors.

This inconsistency could lead to different behavior between the test script and the threshold analysis script. For correctness, empty detectors with a non-zero syndrome should have probability close to zero (1e-30), as this represents an unsatisfiable constraint (no errors can flip the detector, yet it's triggered).

Suggested change

	lines.append("1")
	lines.append("1.0")
	# Empty detector: probability depends on whether the syndrome is consistent.
	# If syndrome[d] == 0, the constraint is satisfied (probability 1.0).
	# If syndrome[d] != 0, the constraint is unsatisfiable (near-zero probability).
	syndrome_bit = int(syndrome[d])
	lines.append("1")
	if syndrome_bit == 0:
	lines.append("1.0")
	else:
	lines.append("1e-30")

[Copilot]

Same bug as in analyze_tropical_threshold.py: inconsistent handling of empty detector factors.

When a detector has no connected errors (empty variable list), the factor value is always set to "1.0" (line 72) regardless of the syndrome value. This should depend on the syndrome:

• If syndrome[d] == 0: assign "1.0" (constraint is satisfied)
• If syndrome[d] != 0: assign "1e-30" (constraint is violated - no errors can flip this detector)

The correct implementation can be found in test_tropical_mwpm_match.py lines 76-79.

Suggested change

	lines.append("1")
	lines.append("1.0")
	syndrome_bit = int(syndrome[d])
	lines.append("1")
	if syndrome_bit == 0:
	lines.append("1.0")
	else:
	lines.append("1e-30")

[Copilot]

The combine_trees function uses recursion with a divide-and-conquer approach (lines 183-187). For a large number of disconnected components, this could potentially hit Python's recursion limit.

For example, with 1000 disconnected components, the recursion depth would be approximately log2(1000) ≈ 10 levels, which is safe. However, if there were thousands of disconnected components (unusual but possible), this could become a concern.

Consider adding a check or comment about the expected maximum number of components, or alternatively, using an iterative approach for combining trees if this becomes a practical issue.

Suggested change

	if len(trees) == 1:
	return trees[0]
	elif len(trees) == 2:
	# Combine two trees
	# Get output indices for each
	def get_output_vars(tree):
	if "tensor_index" in tree:
	return list(nodes[tree["tensor_index"]].vars)
	eins = tree.get("eins", {})
	return eins.get("iy", [])
	out0 = get_output_vars(trees[0])
	out1 = get_output_vars(trees[1])
	combined_out = list(dict.fromkeys(out0 + out1))
	return {
	"args": trees,
	"eins": {"ixs": [out0, out1], "iy": combined_out}
	}
	else:
	# Recursively combine
	mid = len(trees) // 2
	left = combine_trees(trees[:mid])
	right = combine_trees(trees[mid:])
	return combine_trees([left, right])
	"""Combine a list of component trees into a single tree without recursion.
	The combination pattern is effectively the same as the previous
	divide-and-conquer approach, but implemented iteratively to avoid
	any risk of hitting Python's recursion limit if there are many
	disconnected components.
	"""
	# Get output indices for a tree
	def get_output_vars(tree):
	if "tensor_index" in tree:
	return list(nodes[tree["tensor_index"]].vars)
	eins = tree.get("eins", {})
	return eins.get("iy", [])
	trees = list(trees)
	if not trees:
	raise ValueError("combine_trees expects at least one tree")
	if len(trees) == 1:
	return trees[0]
	# Iteratively combine trees in pairs until a single tree remains.
	while len(trees) > 1:
	new_trees = []
	i = 0
	while i < len(trees):
	if i + 1 >= len(trees):
	# Odd tree out, carry to next round unchanged
	new_trees.append(trees[i])
	break
	left_tree = trees[i]
	right_tree = trees[i + 1]
	out0 = get_output_vars(left_tree)
	out1 = get_output_vars(right_tree)
	combined_out = list(dict.fromkeys(out0 + out1))
	new_trees.append(
	{
	"args": [left_tree, right_tree],
	"eins": {"ixs": [out0, out1], "iy": combined_out},
	}
	)
	i += 2
	trees = new_trees
	return trees[0]

[Copilot]

The _find_connected_components function does not handle factors with empty variable lists correctly. If a factor has an empty variable list (no variables), it will not be added to any component and will be silently dropped from the result. This could lead to factors being excluded from the contraction tree.

Consider the case where ixs = [[], [1], [2]]:

• Factor 0 has no variables, so it won't be added to var_to_factors
• Factor 0 will never be unioned with any other factor
• Factor 0 will form its own component with root 0
• However, the logic appears correct - it will create a singleton component for factor 0

Actually, upon closer inspection, the code should handle this correctly because all factors are initialized in the parent array (line 72), and the grouping step (lines 90-95) will include all factors. Each factor with an empty variable list would form its own singleton component, which seems correct.

However, there's a potential issue: What if a factor genuinely has no variables (a constant factor)? The current implementation would create a singleton component for it, which is probably correct, but this edge case should be tested.

[Copilot]

The get_output_vars function (line 168) within combine_trees might fail or behave unexpectedly for certain tree structures. Specifically:

1. If a tree is a leaf node (has "tensor_index"), it returns list(nodes[tree["tensor_index"]].vars) (line 170)
2. Otherwise, it returns eins.get("iy", []) (line 172)

However, if the tree is not a leaf and doesn't have an "eins" key, or if "eins" doesn't have an "iy" key, this will return an empty list []. This could lead to incorrect combined output indices.

For robustness, consider adding validation to ensure that non-leaf nodes have the expected structure, or provide clearer error messages when the tree structure is unexpected.

Suggested change

	eins = tree.get("eins", {})
	return eins.get("iy", [])
	eins = tree.get("eins")
	if not isinstance(eins, dict) or "iy" not in eins:
	raise ValueError(
	"Invalid contraction tree node: non-leaf nodes must have an "
	"'eins' mapping with an 'iy' key specifying output variables."
	)
	return list(eins["iy"])

[Copilot]

Variable observables is not used.

Suggested change

observables = samples[:, -1].astype(np.int32)

[Copilot]

Import of 'pytest' is not used.

Suggested change

import pytest

[Copilot]

Import of 'torch' is not used.

Suggested change

import torch