Merge pull request #4059 from tybug/fix-semibounded-all-children

Fix semi-bounded `all_children` computation

Author: Zac-HD

hypothesis-python/RELEASE.rst

@@ -0,0 +1,3 @@
+RELEASE_TYPE: patch
+
+This patch fixes a rare internal error when using :func:`~hypothesis.strategies.integers` with a high number of examples and certain ``{min, max}_value`` parameters (:pull:`4059`).

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

@@ -2083,11 +2083,17 @@ def draw_integer(
             assert len(weights) == width
 
         if forced is not None and (min_value is None or max_value is None):
-            # We draw `forced=forced - shrink_towards` here internally. If that
-            # grows larger than a 128 bit signed integer, we can't represent it.
+            # We draw `forced=forced - shrink_towards` here internally, after clamping.
+            # If that grows larger than a 128 bit signed integer, we can't represent it.
             # Disallow this combination for now.
             # Note that bit_length() = 128 -> signed bit size = 129.
-            assert (forced - shrink_towards).bit_length() < 128
+            _shrink_towards = shrink_towards
+            if min_value is not None:
+                _shrink_towards = max(min_value, _shrink_towards)
+            if max_value is not None:
+                _shrink_towards = min(max_value, _shrink_towards)
+
+            assert (forced - _shrink_towards).bit_length() < 128
         if forced is not None and min_value is not None:
             assert min_value <= forced
         if forced is not None and max_value is not None:

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

@@ -285,22 +285,18 @@ def all_children(ir_type, kwargs):
                         continue
                     yield n
         else:
-            # hard case: only one bound was specified. Here we probe either upwards
-            # or downwards with our full 128 bit generation, but only half of these
-            # (plus one for the case of generating zero) result in a probe in the
-            # direction we want. ((2**128 - 1) // 2) + 1 == a range of 2 ** 127.
-            #
-            # strictly speaking, I think this is not actually true: if
-            # max_value > shrink_towards then our range is ((-2**127) + 1, max_value),
-            # and it only narrows when max_value < shrink_towards. But it
-            # really doesn't matter for this case because (even half) unbounded
-            # integers generation is hit extremely rarely.
             assert (min_value is None) ^ (max_value is None)
+            # hard case: only one bound was specified. Here we probe in 128 bits
+            # around shrink_towards, and discard those above max_value or below
+            # min_value respectively.
+            shrink_towards = kwargs["shrink_towards"]
             if min_value is None:
-                yield from range(max_value - (2**127) + 1, max_value)
+                shrink_towards = min(max_value, shrink_towards)
+                yield from range(shrink_towards - (2**127) + 1, max_value)
             else:
                 assert max_value is None
-                yield from range(min_value, min_value + (2**127) - 1)
+                shrink_towards = max(min_value, shrink_towards)
+                yield from range(min_value, shrink_towards + (2**127) - 1)
 
     if ir_type == "boolean":
         p = kwargs["p"]

hypothesis-python/tests/conjecture/test_forced.py

@@ -225,3 +225,23 @@ def test_forced_floats_with_nan(random, sign, min_value, max_value):
     # trying to use float clampers that didn't exist when drawing.
     data = fresh_data(random=random)
     data.draw_float(min_value=min_value, max_value=max_value, forced=sign * math.nan)
+
+
+@given(st.data())
+def test_forced_with_large_magnitude_integers(data):
+    bound_offset = data.draw(st.integers(min_value=0))
+    # forced_offset = bound_offset + st.integers(min_value=0) may look cleaner, but
+    # has subtly different maximum value semantics as it is twice the range of a
+    # single draw
+    forced_offset = data.draw(st.integers(min_value=bound_offset))
+
+    half_range = 2**127 + 1
+    cd = fresh_data()
+    cd.draw_integer(
+        min_value=half_range + bound_offset, forced=half_range + forced_offset
+    )
+
+    cd = fresh_data()
+    cd.draw_integer(
+        max_value=-(half_range + bound_offset), forced=-(half_range + forced_offset)
+    )

hypothesis-python/tests/conjecture/test_ir.py

@@ -33,6 +33,7 @@
 
 from tests.common.debug import minimal
 from tests.conjecture.common import (
+    draw_integer_kwargs,
     draw_value,
     fresh_data,
     ir_nodes,
@@ -272,13 +273,33 @@ def test_compute_max_children_and_all_children_agree(ir_type_and_kwargs):
 # compute_max_children, because they by necessity require iterating over 2**127
 # or more elements. We do the not great approximation of checking just the first
 # element is what we expect.
-@pytest.mark.parametrize(
-    "min_value, max_value, first",
-    [(None, None, -(2**127) + 1), (None, 42, (-(2**127) + 1) + 42), (42, None, 42)],
-)
-def test_compute_max_children_unbounded_integer_ranges(min_value, max_value, first):
-    kwargs = {"min_value": min_value, "max_value": max_value, "weights": None}
-    assert first == next(all_children("integer", kwargs))
+
+
+@pytest.mark.parametrize("use_min_value", [True, False])
+@pytest.mark.parametrize("use_max_value", [True, False])
+def test_compute_max_children_unbounded_integer_ranges(use_min_value, use_max_value):
+    @given(
+        draw_integer_kwargs(
+            use_min_value=use_min_value,
+            use_max_value=use_max_value,
+            use_weights=use_min_value and use_max_value,
+        )
+    )
+    def f(kwargs):
+        if kwargs["min_value"] is not None:
+            expected = kwargs["min_value"]
+        else:
+            offset = (
+                0
+                if kwargs["max_value"] is None
+                else min(kwargs["max_value"], kwargs["shrink_towards"])
+            )
+            expected = offset - (2**127) + 1
+
+        first = next(all_children("integer", kwargs))
+        assert expected == first, (expected, first)
+
+    f()
 
 
 @given(st.randoms())