Address review: reset bound cursor in remove_unfinished, simplify test

- Reset _next_bound_idx in remove_unfinished() so discarded pending
  bound points can be asked again (previously they were silently
  retired and the domain corners never sampled).
- Replace the unreachable 'raise StopIteration' with a RuntimeError;
  StopIteration would surface as a confusing PEP 479 RuntimeError from
  the generator in _ask_and_tell_pending anyway.
- Drop the brittle message-matching try/except in
  _try_adding_pending_point_to_simplex; _is_known_point makes that
  path unreachable and its cleanup was incomplete.
- Scale _is_known_point tolerance with coordinate magnitude instead of
  flooring at an absolute 1e-10, which was too coarse for tiny domains.
- Simplify the regression test: derive the drifted hull vertex instead
  of magic constants, use pytest.importorskip, and remove the half that
  re-installed the old implementation to assert it fails.
- Add a regression test for the remove_unfinished bound-point reset.

Author: basnijholt

adaptive/learner/learnerND.py

@@ -613,8 +613,11 @@ def _is_known_point(self, point):
         if point in self.data or point in self.pending_points:
             return True
 
+        # Scale the tolerance with the coordinate magnitude so that float
+        # round-trip drift (e.g. through a dataframe) is matched in domains
+        # of any size.
         tolerances = [
-            max(self._bound_match_tol, self._bound_match_tol * (hi - lo))
+            self._bound_match_tol * max(abs(lo), abs(hi), hi - lo)
             for lo, hi in self._bbox
         ]
 
@@ -678,12 +681,7 @@ def _try_adding_pending_point_to_simplex(self, point, simplex):
             self._subtriangulations[simplex] = Triangulation(vertices)
 
         self._pending_to_simplex[point] = simplex
-        try:
-            return self._subtriangulations[simplex].add_point(point)
-        except ValueError as exc:
-            if str(exc) == "Point already in triangulation.":
-                self._pending_to_simplex.pop(point, None)
-            raise
+        return self._subtriangulations[simplex].add_point(point)
 
     def _update_subsimplex_losses(self, simplex, new_subsimplices):
         loss = self._losses[simplex]
@@ -718,7 +716,9 @@ def _ask_bound_point(self):
             self.tell_pending(new_point)
             return new_point, np.inf
 
-        raise StopIteration
+        # Unreachable: _ask only calls this method when _bounds_available,
+        # which guarantees an unknown bound point at index >= _next_bound_idx.
+        raise RuntimeError("No bound points available to ask.")
 
     def _ask_point_without_known_simplices(self):
         assert not self._bounds_available
@@ -797,7 +797,7 @@ def _bounds_available(self):
 
     def _ask(self):
         if self._bounds_available:
-            return self._ask_bound_point()  # O(1)
+            return self._ask_bound_point()  # O(N) worst case, amortized O(1)
 
         if self.tri is None:
             # All bound points are pending or have been evaluated, but we do not
@@ -967,6 +967,9 @@ def remove_unfinished(self):
         self.pending_points = set()
         self._subtriangulations = {}
         self._pending_to_simplex = {}
+        # Discarded pending points may include bound points that were already
+        # consumed by _ask_bound_point; rescan them so they can be asked again.
+        self._next_bound_idx = 0
 
     ##########################
     # Plotting related stuff #

adaptive/tests/unit/test_learnernd_integration.py

@@ -1,11 +1,9 @@
 import math
 
-import numpy as np
 import pytest
 from scipy.spatial import ConvexHull
 
 from adaptive.learner import LearnerND
-from adaptive.learner.learner1D import with_pandas
 from adaptive.learner.learnerND import curvature_loss_function
 from adaptive.runner import BlockingRunner
 from adaptive.runner import simple as SimpleRunner
@@ -58,11 +56,9 @@ def test_learnerND_log_works():
     # furthermore the last two points that were asked should be in this simplex
 
 
-@pytest.mark.skipif(not with_pandas, reason="pandas is not installed")
-def test_learnerND_resume_after_loading_dataframe_convex_hull(monkeypatch):
-    from types import MethodType
-
-    import pandas
+def test_learnerND_resume_after_loading_dataframe_convex_hull():
+    # Regression test for https://github.com/python-adaptive/adaptive/issues/470
+    pandas = pytest.importorskip("pandas")
 
     hull_points = [
         (4.375872112626925, 8.917730007820797),
@@ -71,54 +67,19 @@ def test_learnerND_resume_after_loading_dataframe_convex_hull(monkeypatch):
         (9.636627605010293, 3.8344151882577773),
     ]
 
-    data_points = [
-        (4.375872112626925, 8.917730007820797),
-        (4.236547993389047, 6.458941130666561),
-        (6.027633760716439, 5.448831829968968),
-        (9.636627605086398, 3.834415188269945),
-        (0.7103605819788694, 0.8712929970154071),
-        (0.2021839744032572, 8.32619845547938),
-        (7.781567509498505, 8.700121482468191),
-    ]
+    # Simulate float drift from a dataframe round-trip: one hull vertex is
+    # off by 1e-10, so exact membership checks miss it and the learner used
+    # to re-ask it, crashing with "Point already in triangulation.".
+    drifted = tuple(c + 1e-10 for c in hull_points[-1])
+    data_points = [*hull_points[:-1], drifted, (7.0, 6.0)]
 
     df = pandas.DataFrame(data_points, columns=["x", "y"])
     df["value"] = df["x"] + df["y"]
 
-    hull = ConvexHull(hull_points)
-
     def some_f(xy):
         return xy[0] + xy[1]
 
-    learner_old = LearnerND(some_f, hull)
-    learner_old.load_dataframe(
-        df,
-        with_default_function_args=False,
-        point_names=("x", "y"),
-        value_name="value",
-    )
-
-    def old_ask_bound_point(self):
-        new_point = next(
-            p
-            for p in self._bounds_points
-            if p not in self.data and p not in self.pending_points
-        )
-        self.tell_pending(new_point)
-        return new_point, np.inf
-
-    learner_old._ask_bound_point = MethodType(old_ask_bound_point, learner_old)
-
-    def naive_is_known_point(self, point):
-        point = tuple(map(float, point))
-        return point in self.data or point in self.pending_points
-
-    learner_old._is_known_point = MethodType(naive_is_known_point, learner_old)
-    learner_old._bound_match_tol = 0.0
-
-    with pytest.raises(ValueError):
-        BlockingRunner(learner_old, npoints_goal=len(df) + 1)
-
-    learner = LearnerND(some_f, hull)
+    learner = LearnerND(some_f, ConvexHull(hull_points))
     learner.load_dataframe(
         df,
         with_default_function_args=False,
@@ -129,3 +90,14 @@ def naive_is_known_point(self, point):
     target = len(df) + 1
     BlockingRunner(learner, npoints_goal=target)
     assert learner.npoints >= target
+
+
+def test_learnerND_remove_unfinished_reasks_bound_points():
+    learner = LearnerND(ring_of_fire, bounds=[(-1, 1), (-1, 1)])
+    points, _ = learner.ask(4)
+    assert set(points) == set(learner._bounds_points)
+
+    # Discarding the pending bound points must make them available again.
+    learner.remove_unfinished()
+    points, _ = learner.ask(4)
+    assert set(points) == set(learner._bounds_points)