Fix mergetree bug

[ishikaghosh2201]

Description

Fixes an IndexError in ReebGraph.smoothing_and_maps that crashed
whenever a Reeb graph (or MergeTree) contained an infinite-valued
vertex — most commonly a MergeTree's v_inf root.

Root cause: the delta-shift step, used to find "the edge slightly below"
a critical value, assumed every critical value has a finite neighbour to
shift toward. That assumption breaks at +/- infinity, since inf - eps
is still inf, so the shifted slice collapsed to the isolated infinite
vertex itself with no overlap below it.

Fix:

• ReebGraph.smoothing_and_maps now smooths only the finite part of the
  graph, then reattaches infinite vertices afterwards, reconnected to
  whichever finite component(s) end up adjacent to them post-smoothing.
  Handles both +inf and -inf, and multiple branches attaching
  directly to an infinite vertex without a shared merge point below it.
• MergeTree.smoothing_and_maps is now a thin re-wrap of the (already
  correct) inherited method, rather than duplicating that logic.
• Bare lower_vert[0] / upper_vert[0] indexing replaced with explicit
  length checks and a clear ValueError, as a backstop for any other
  unhandled degenerate case.

Motivation and Context

MergeTree.smoothing()/smoothing_and_maps() crashed on any tree with
its (always-present) v_inf root, making smoothing unusable for
MergeTree objects entirely. This is linked to issues #102 and #100.

How has this been tested?

• Verified against the original bug report notebook's minimal repro
  (3-node path + v_inf), randomMergeTree, multiple eps values, a
  shared-merge-point two-branch tree, and two branches attaching
  directly to v_inf with no shared merge point below them.
• Verified the plain ReebGraph (non-MergeTree) case with a manually
  added +inf node, plus -inf and combined +inf/-inf graphs.
• Confirmed MapperGraph and Interleave are unaffected: both
  structurally exclude infinite function values, so the new code path
  is unreachable through either.
• New tests added in tests/test_reeb_class.py and
  tests/test_merge_tree.py.
• make tests: all passing.

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to not work as expected)

Checklist

• I have incremented the version number in the pyproject.toml file if a new version needs to be pushed to pypi. Note that if the number isn't incremented, the package will not be pushed to pypi, which is useful if this PR is only for updating documentation.
• My code follows the code style of this project and I have run make format to clean up the code with black.
• My change requires a change to the documentation. I have updated the documentation as necessary and compiled locally to ensure it is clean.
• I have added tests to cover my changes, and all new and existing tests passed (run make tests).

[Copilot]

🟡 Changes recommended

Unresolved critical findings affect smoothing edge cases, label preservation, and interleaving bounds.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

Fixes smoothing failures for Reeb and MergeTree graphs containing infinite-valued vertices, with added regression tests.

Changes:

• Smooth finite subgraphs and reattach infinite vertices.
• Preserve MergeTree output handling.
• Adjust interleaving search bounds.
• Add smoothing and labeling tests.

File summaries

File	Summary
tests/test_reeb_class.py	Infinite-node smoothing regression tests
tests/test_merge_tree.py	MergeTree smoothing and label tests
cereeberus/cereeberus/reeb/reebgraph.py	Infinite-vertex smoothing and validation
cereeberus/cereeberus/reeb/merge.py	MergeTree smoothing wrapper
cereeberus/cereeberus/distance/interleave.py	Distance-search bound changes

Review details

Suppressed comments (6)

cereeberus/cereeberus/reeb/reebgraph.py:973

• The set(...) deduplicates attachment vertices and creates only one tv -> inf_v edge regardless of how many parallel original edges (u, inf_v) exist. Since ReebGraph is a MultiDiGraph, this collapses parallel edges and makes every corresponding map_E entry point to the same edge, changing the graph topology. Add one output edge per original edge key and record each newly created key.

                for tv in set(v for verts in attach_up.values() for v in verts):
                    R_eps.add_edge(tv, inf_v, reset_pos=False)
                for u in preds:
                    new_edges_u = [
                        (tv, inf_v, R_eps.number_of_edges(tv, inf_v) - 1)

cereeberus/cereeberus/reeb/reebgraph.py:985

• The same deduplication collapses parallel original edges (inf_v, w) into one output edge, so the lower-side edge map loses multiplicity as well. Preserve one newly created inf_v -> bv edge for each original edge key, rather than adding edges once per distinct endpoint.

                for bv in set(v for verts in attach_down.values() for v in verts):
                    R_eps.add_edge(inf_v, bv, reset_pos=False)
                for w in succs:
                    new_edges_w = [
                        (inf_v, bv, R_eps.number_of_edges(inf_v, bv) - 1)

cereeberus/cereeberus/reeb/reebgraph.py:968

• These neighbor lists can include another infinite vertex. For a valid -inf -> +inf edge, the other endpoint is not part of the finite comp_top/comp_bottom maps (and may not yet be present in map_V), so this lookup raises KeyError. Defer infinite-to-infinite edges and reconnect/map them separately after processing finite attachments.

                preds = list(self.predecessors(inf_v))
                attach_up = {u: comp_top[map_V[u]] for u in preds}

cereeberus/cereeberus/reeb/reebgraph.py:963

• components is recomputed after each infinite vertex is added, so a second +inf (or -inf) vertex sharing a finite component sees the earlier infinite vertex as that component's top (or bottom). It is then connected to the previous infinite vertex instead of the finite boundary, turning parallel root branches into a chain. Compute these boundaries from a finite-only snapshot before adding any infinite vertices.

                components = list(nx.weakly_connected_components(R_eps))
                comp_top = {}
                comp_bottom = {}
                for comp in components:
                    tops = [v for v in comp if R_eps.up_degree(v) == 0]
                    bottoms = [v for v in comp if R_eps.down_degree(v) == 0]
                    for v in comp:
                        comp_top[v] = tops
                        comp_bottom[v] = bottoms

cereeberus/cereeberus/reeb/reebgraph.py:949

• Because the finite graph is built without the infinite nodes, _get_next_internal_vert_name cannot reserve their names. If an infinite vertex is named like an internal vertex, for example ('reeb_auto', 0), smoothing can generate that name in R_eps and then add_node(inf_v, ...) raises a duplicate-vertex ValueError. Reserve all original infinite names or otherwise choose output names disjoint from them.

        inf_nodes = [v for v in self.nodes if np.isinf(self.f[v])]
        if inf_nodes:
            # smooth the finite part, then reattach inf vertices afterward
            finite_nodes = [v for v in self.nodes if v not in inf_nodes]
            f_finite = {v: self.f[v] for v in finite_nodes}
            G_finite = ReebGraph(self.subgraph(finite_nodes), f_finite)

tests/test_reeb_class.py:283

• This regression test only asserts that top has no outgoing edge. An isolated top also satisfies that assertion, so the test would pass even if the new reattachment logic dropped the connection to the finite component; assert that top has an incoming edge as well.

        self.assertEqual(R_eps.up_degree('top'), 0)

• Files reviewed: 5/5 changed files
• Comments generated: 4
• Review effort level: Lite

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.