From 10b8235a70cb65d992aee6ead0715aeba87f1c86 Mon Sep 17 00:00:00 2001 From: ishikaghosh2201 <112980412+ishikaghosh2201@users.noreply.github.com> Date: Fri, 18 Sep 2026 11:48:27 -0400 Subject: [PATCH 1/3] Fix ValueError from ULP-adjacent tied filtration values in computeReeb --- cereeberus/cereeberus/compute/computereeb.py | 41 +++++- tests/test_computereeb.py | 139 +++++++++++++++++++ 2 files changed, 178 insertions(+), 2 deletions(-) create mode 100644 tests/test_computereeb.py diff --git a/cereeberus/cereeberus/compute/computereeb.py b/cereeberus/cereeberus/compute/computereeb.py index e27a74d4..65a0d569 100644 --- a/cereeberus/cereeberus/compute/computereeb.py +++ b/cereeberus/cereeberus/compute/computereeb.py @@ -5,6 +5,12 @@ from ..reeb.lowerstar import LowerStar from .unionfind import UnionFind +# Tolerance for treating two filtration values as float64-indistinguishable +# (i.e. within ~1 ULP of each other, the scale of floating-point rounding +# noise. It is not intended to merge genuinely distinct nearby values). +_ULP_RTOL = 1e-9 +_ULP_ATOL = 1e-12 + def is_face(sigma, tau): """ @@ -48,6 +54,30 @@ def get_levelset_components(L): return components +def _merge_close_values(sorted_vals, rtol=_ULP_RTOL, atol=_ULP_ATOL): + """ + Snap chains of float64-indistinguishable values together. + + Two floats are considered the same point if their midpoint rounds back + to one of them (no float64 value fits strictly between them), or more + generally if they fall within rtol/atol of each other. This only + catches floating-point rounding noise (~1 ULP), never genuinely + distinct nearby values from real data. + """ + out = list(sorted_vals) + i, n = 0, len(out) + while i < n - 1: + j = i + while j + 1 < n and np.isclose(out[j + 1], out[i], rtol=rtol, atol=atol): + j += 1 + if j > i: + rep = out[i] + for k in range(i, j + 1): + out[k] = rep + i = j + 1 + return out + + def computeReeb(K: LowerStar, verbose=False): """Computes the Reeb graph of a Lower Star Simplicial Complex K. @@ -78,6 +108,8 @@ def computeReeb(K: LowerStar, verbose=False): # Group vertices that share the same filtration value into batches. # A horizontal edge (both endpoints at the same height) must be processed # within one batch so it properly merges its endpoints into a single Reeb node. + snapped_vals = _merge_close_values([f for _, f in funcVals]) + funcVals = [(i, snapped_vals[idx]) for idx, (i, _) in enumerate(funcVals)] grouped = [ (filt, list(grp)) for filt, grp in _groupby(funcVals, key=lambda x: x[1]) ] @@ -129,9 +161,14 @@ def _dedup(lst): simplex, s_filt = s[0], s[1] if len(simplex) <= 1: continue - if s_filt > filt: + if s_filt > filt and not np.isclose( + s_filt, filt, rtol=_ULP_RTOL, atol=_ULP_ATOL + ): all_upper.append(simplex) - elif all(K.filtration([u]) == filt for u in simplex): + elif all( + np.isclose(K.filtration([u]), filt, rtol=_ULP_RTOL, atol=_ULP_ATOL) + for u in simplex + ): all_horizontal.append(simplex) else: all_lower_nonhoriz.append(simplex) diff --git a/tests/test_computereeb.py b/tests/test_computereeb.py new file mode 100644 index 00000000..98ef9856 --- /dev/null +++ b/tests/test_computereeb.py @@ -0,0 +1,139 @@ +import math +import unittest + +import numpy as np + +from cereeberus.reeb.lowerstar import LowerStar +from cereeberus.compute.computereeb import computeReeb, _merge_close_values + + +class TestMergeCloseValues(unittest.TestCase): + """Unit tests for the _merge_close_values helper directly.""" + + def test_ulp_adjacent_values_are_merged(self): + H = 1.0 + H2 = math.nextafter(H, math.inf) # 1 ULP above H + out = _merge_close_values([0.0, H, H2, 5.0]) + self.assertEqual(out[1], out[2]) # H and H2 snapped to the same value + + def test_genuinely_distinct_values_are_not_merged(self): + vals = [0.0, 1.0, 2.0, 3.0] + out = _merge_close_values(vals) + self.assertEqual(out, vals) + + def test_chain_of_three_close_values_all_merge_to_first(self): + H = 1.0 + H2 = math.nextafter(H, math.inf) + H3 = math.nextafter(H2, math.inf) + out = _merge_close_values([H, H2, H3]) + self.assertEqual(out, [H, H, H]) + + def test_values_far_apart_by_absolute_gap_not_merged(self): + # rtol alone would flag these as close at large magnitude; atol/rtol + # combination here should still treat a real gap as real. + out = _merge_close_values([1000.0, 1000.1]) + self.assertEqual(out, [1000.0, 1000.1]) + + +class TestComputeReebTiedHeights(unittest.TestCase): + """Regression tests for the ULP-adjacent tied-heights crash. + + computeReeb raised 'ValueError: The vertex X must be in the Reeb graph...' when two + distinct filtration values were 1 float64 ULP apart, causing the + half-edge sentinel midpoint (now_min + now_max) / 2 to round to + exactly one of them and spuriously trigger the tie-collapse branch. + """ + + def _build_ulp_adjacent_case(self): + """Minimal 5-vertex hand-traceable repro of the ULP-collision bug. + + Two triangles sharing an edge (2,3); vertices 2 and 3 sit 1 ULP + apart in height, close enough that the algorithm's own midpoint + calculation could not previously tell them apart from a sentinel. + """ + H = 1.0 + H2 = math.nextafter(H, math.inf) # 1 ULP above H + K = LowerStar() + K.insert([0, 2, 3]) + K.insert([1, 2, 3]) + K.insert([2, 3, 4]) + K.assign_filtration(0, 0.0) + K.assign_filtration(1, 0.0) + K.assign_filtration(2, H) + K.assign_filtration(3, H2) + K.assign_filtration(4, H + 1.0) + return K + + def test_ulp_adjacent_heights_do_not_crash(self): + K = self._build_ulp_adjacent_case() + try: + R = computeReeb(K) + except ValueError as e: + self.fail(f"computeReeb raised ValueError on ULP-adjacent heights: {e}") + + # Structural sanity: should be a connected tree (no cycles introduced). + self.assertEqual(R.number_of_edges(), R.number_of_nodes() - 1) + + def test_exact_tie_still_works(self): + """Regression guard: genuine exact ties must still collapse correctly + (this is the originally-fixed, closed issue -- must not regress).""" + K = LowerStar() + K.insert([0, 2, 3]) + K.insert([1, 2, 3]) + K.insert([2, 3, 4]) + K.assign_filtration(0, 0.0) + K.assign_filtration(1, 0.0) + K.assign_filtration(2, 1.0) + K.assign_filtration(3, 1.0) # exact tie with vertex 2 + K.assign_filtration(4, 2.0) + + R = computeReeb(K) + self.assertEqual(R.number_of_edges(), R.number_of_nodes() - 1) + + def test_docstring_example_unchanged(self): + """Non-degenerate case with no near-ties: output must be identical + to pre-fix behavior.""" + K = LowerStar() + K.insert([0, 1, 2]) + K.insert([1, 3]) + K.insert([2, 3]) + K.assign_filtration([0], 0.0) + K.assign_filtration([1], 3.0) + K.assign_filtration([2], 5.0) + K.assign_filtration([3], 7) + + R = computeReeb(K) + self.assertEqual(R.number_of_nodes(), 10) + self.assertEqual(R.number_of_edges(), 10) + + +@unittest.skipUnless( + __import__("importlib").util.find_spec("trimesh") is not None, + "trimesh not installed; skipping real-mesh regression test", +) +class TestComputeReebTiedHeightsRealMesh(unittest.TestCase): + """Real-world confirmation on a symmetric mesh (optional; requires trimesh). + + Icospheres reliably produce vertices whose true height is identical by + construction but that land 1 ULP apart due to differing subdivision + paths. + """ + + def test_icosphere_subdivisions_2_does_not_crash(self): + import trimesh + + mesh = trimesh.creation.icosphere(subdivisions=2) + K = LowerStar() + for tri in mesh.faces: + K.insert([int(v) for v in tri]) + for i, v in enumerate(mesh.vertices): + K.assign_filtration(i, float(v[2])) + + R = computeReeb(K, verbose=False) + self.assertEqual(R.number_of_nodes(), 81) + self.assertEqual(R.number_of_edges(), 80) + self.assertEqual(R.number_of_edges(), R.number_of_nodes() - 1) + + +if __name__ == "__main__": + unittest.main() \ No newline at end of file From c9fe1dd1d074388ef721dd736e90de38a22aa967 Mon Sep 17 00:00:00 2001 From: Ishika Ghosh Date: Mon, 28 Sep 2026 10:38:35 -0400 Subject: [PATCH 2/3] Update pulp version constraint in requirements.txt --- requirements.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/requirements.txt b/requirements.txt index e1fa66f1..a462920b 100644 --- a/requirements.txt +++ b/requirements.txt @@ -3,7 +3,7 @@ networkx matplotlib pandas scipy -pulp +pulp<4 gudhi sphinx nbsphinx From c57255c01bcc30903b89ef73101d2f5e2bfdb085 Mon Sep 17 00:00:00 2001 From: Ishika Ghosh Date: Mon, 28 Sep 2026 10:38:57 -0400 Subject: [PATCH 3/3] Update pulp dependency version constraint --- pyproject.toml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index d70d00e1..f21a1d91 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -20,7 +20,7 @@ dependencies = ["numpy", "matplotlib", "pandas", "scipy", - "pulp", + "pulp<4", "gudhi", "scikit-learn" ] @@ -34,4 +34,4 @@ cereeberus = ["data"] [project.urls] repository = "https://github.com/MunchLab/ceREEBerus" homepage = "https://munchlab.github.io/ceREEBerus/" -documentation = "https://munchlab.github.io/ceREEBerus/" \ No newline at end of file +documentation = "https://munchlab.github.io/ceREEBerus/"