diff --git a/hypothesis-python/RELEASE.rst b/hypothesis-python/RELEASE.rst new file mode 100644 index 0000000000..73d5dcf621 --- /dev/null +++ b/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`). diff --git a/hypothesis-python/src/hypothesis/internal/conjecture/data.py b/hypothesis-python/src/hypothesis/internal/conjecture/data.py index 6c68b441d6..3f81f262eb 100644 --- a/hypothesis-python/src/hypothesis/internal/conjecture/data.py +++ b/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: diff --git a/hypothesis-python/src/hypothesis/internal/conjecture/datatree.py b/hypothesis-python/src/hypothesis/internal/conjecture/datatree.py index ead0157395..c5602a35f4 100644 --- a/hypothesis-python/src/hypothesis/internal/conjecture/datatree.py +++ b/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"] diff --git a/hypothesis-python/tests/conjecture/test_forced.py b/hypothesis-python/tests/conjecture/test_forced.py index 97994434d8..cb22c09359 100644 --- a/hypothesis-python/tests/conjecture/test_forced.py +++ b/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) + ) diff --git a/hypothesis-python/tests/conjecture/test_ir.py b/hypothesis-python/tests/conjecture/test_ir.py index 2c725ac431..142c577584 100644 --- a/hypothesis-python/tests/conjecture/test_ir.py +++ b/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())