Merge pull request #4279 from jobh/nan-shrink

Shrink NaN to canonical form

Author: Zac-HD

hypothesis-python/RELEASE.rst

@@ -0,0 +1,3 @@
+RELEASE_TYPE: patch
+
+Improve shrinking of non-standard NaN float values (:issue:`4277`).

hypothesis-python/src/hypothesis/internal/conjecture/choice.py

@@ -417,7 +417,7 @@ def choice_to_index(choice: ChoiceT, kwargs: ChoiceKwargsT) -> int:
             to_order=intervals.index_from_char_in_shrink_order,
         )
     elif isinstance(choice, float):
-        sign = int(sign_aware_lte(choice, -0.0))
+        sign = int(math.copysign(1.0, choice) < 0)
         return (sign << 64) | float_to_lex(abs(choice))
     else:
         raise NotImplementedError

hypothesis-python/src/hypothesis/internal/conjecture/shrinking/common.py

@@ -34,7 +34,7 @@ def __init__(
         self.name = name
 
         self.__predicate = predicate
-        self.__seen = set()
+        self.__seen = {self.make_canonical(self.current)}
         self.debugging_enabled = debug
 
     @property
@@ -107,39 +107,46 @@ def run(self):
             self.run_step()
         self.debug("COMPLETE")
 
-    def incorporate(self, value):
+    def consider(self, value):
         """Try using ``value`` as a possible candidate improvement.
 
-        Return True if it works.
+        Return True if self.current is canonically equal to value after the call, either because
+        the value was incorporated as an improvement or because it had that value already.
         """
         value = self.make_immutable(value)
+        self.debug(f"considering {value!r}")
+        canonical = self.make_canonical(value)
+        if canonical == self.make_canonical(self.current):
+            return True
+        if canonical in self.__seen:
+            return False
+        self.__seen.add(canonical)
         self.check_invariants(value)
         if not self.left_is_better(value, self.current):
-            if value != self.current and (value == value):
-                self.debug(f"Rejected {value!r} as worse than {self.current=}")
-            return False
-        if value in self.__seen:
+            self.debug(f"Rejected {value!r} as no better than {self.current=}")
             return False
-        self.__seen.add(value)
         if self.__predicate(value):
             self.debug(f"shrinking to {value!r}")
             self.changes += 1
             self.current = value
             return True
-        return False
+        else:
+            self.debug(f"Rejected {value!r} not satisfying predicate")
+            return False
 
-    def consider(self, value):
-        """Returns True if make_immutable(value) == self.current after calling
-        self.incorporate(value)."""
-        self.debug(f"considering {value}")
-        value = self.make_immutable(value)
-        if value == self.current:
-            return True
-        return self.incorporate(value)
+    def make_canonical(self, value):
+        """Convert immutable value into a canonical and hashable, but not necessarily equal,
+        representation of itself.
+
+        This representation is used only for tracking already-seen values, not passed to the
+        shrinker.
+
+        Defaults to just returning the (immutable) input value.
+        """
+        return value
 
     def make_immutable(self, value):
-        """Convert value into an immutable (and hashable) representation of
-        itself.
+        """Convert value into an immutable representation of itself.
 
         It is these immutable versions that the shrinker will work on.
 

hypothesis-python/src/hypothesis/internal/conjecture/shrinking/floats.py

@@ -14,24 +14,23 @@
 from hypothesis.internal.conjecture.floats import float_to_lex
 from hypothesis.internal.conjecture.shrinking.common import Shrinker
 from hypothesis.internal.conjecture.shrinking.integer import Integer
-from hypothesis.internal.floats import MAX_PRECISE_INTEGER
+from hypothesis.internal.floats import MAX_PRECISE_INTEGER, float_to_int
 
 
 class Float(Shrinker):
     def setup(self):
-        self.NAN = math.nan
         self.debugging_enabled = True
 
-    def make_immutable(self, f):
-        f = float(f)
+    def make_canonical(self, f):
         if math.isnan(f):
-            # Always use the same NAN so it works properly in self.seen
-            f = self.NAN
+            # Distinguish different NaN bit patterns, while making each equal to itself.
+            # Wrap in tuple to avoid potential collision with (huge) finite floats.
+            return ("nan", float_to_int(f))
         return f
 
     def check_invariants(self, value):
-        # We only handle positive floats because we encode the sign separately
-        # anyway.
+        # We only handle positive floats (including NaN) because we encode the sign
+        # separately anyway.
         assert not (value < 0)
 
     def left_is_better(self, left, right):

hypothesis-python/tests/conjecture/test_float_encoding.py

@@ -8,6 +8,7 @@
 # v. 2.0. If a copy of the MPL was not distributed with this file, You can
 # obtain one at https://mozilla.org/MPL/2.0/.
 
+import math
 import sys
 
 import pytest
@@ -16,7 +17,7 @@
 from hypothesis.internal.compat import ceil, extract_bits, floor
 from hypothesis.internal.conjecture import floats as flt
 from hypothesis.internal.conjecture.engine import ConjectureRunner
-from hypothesis.internal.floats import float_to_int
+from hypothesis.internal.floats import SIGNALING_NAN, float_to_int
 
 EXPONENTS = list(range(flt.MAX_EXPONENT + 1))
 assert len(EXPONENTS) == 2**11
@@ -200,3 +201,9 @@ def test_reject_out_of_bounds_floats_while_shrinking():
     kwargs = {"min_value": 103.0}
     g = minimal_from(103.1, lambda x: x >= 100, kwargs=kwargs)
     assert g == 103.0
+
+
+@pytest.mark.parametrize("nan", [-math.nan, SIGNALING_NAN, -SIGNALING_NAN])
+def test_shrinks_to_canonical_nan(nan):
+    shrunk = minimal_from(nan, math.isnan)
+    assert float_to_int(shrunk) == float_to_int(math.nan)

hypothesis-python/tests/cover/test_shrink_budgeting.py

@@ -20,7 +20,7 @@
     [
         (Integer, 2**16),
         (Integer, int(sys.float_info.max)),
-        (Ordering, [[100] * 10]),
+        (Ordering, [(100,) * 10]),
         (Ordering, [i * 100 for i in (range(5))]),
         (Ordering, [i * 100 for i in reversed(range(5))]),
     ],