Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Not limiting the width of float elements (less than double precision) can cause infinities in the tensor, even if allow_infinity is False. #20

Closed
qthequartermasterman opened this issue Apr 26, 2024 · 4 comments · Fixed by #21
Assignees
Labels
bug Something isn't working

Comments

@qthequartermasterman
Copy link
Owner

This bug report is courtesy of @ringohoffman.

Describe the bug
Not limiting the width of float elements (less than double precision) can cause infinities in the tensor, even if allow_infinity is False.

If the floats strategy doesn't have width set, it's generating floats that are outside of the float32 range (i.e. 1e308 (max for fp64) is greater than the max value for fp32). When those values are passed to numpy internally, they're coerced to inf.

To Reproduce

 @hypothesis.given(
        no_inf_tensor=hypothesis_torch.tensor_strategy(
            dtype=torch.float16,
            shape=(1, 10, 20),
            elements=st.floats(allow_infinity=False, allow_nan=False, allow_subnormal=False),
        ),
    )
    def test_no_infinities_generated(self, no_inf_tensor: torch.Tensor) -> None:
        """Test that no infinities are generated."""
        self.assertFalse(torch.isinf(no_inf_tensor).any())
        self.assertFalse(torch.isnan(no_inf_tensor).any())

Expected behavior
A clear and concise description of what you expected to happen.

The generated tensors should never have infinities if allow_infinity is disabled in the elements strategy.

@qthequartermasterman qthequartermasterman added the bug Something isn't working label Apr 26, 2024
@qthequartermasterman
Copy link
Owner Author

Two potential solutions that I've looked at, but don't like are:

  1. Instead of using float as the type to pass to hypothesis.extra.numpy.arrays, use the appropriate numpy dtype. This has the issue that numpy raises an error instead of coercing things to infinity silently. Catching this error and then dismissing the bad example that was generated has huge performance penalties.
  2. Filtering the elements strategy to only allow values that can be exactly represented before passing the strategy to numpy. This works and is faster than solution (1), but still has performance penalties.

Right now, if I had to push out a patch immediately, I would go with (2). I am convinced there is a better way by inspecting the elements strategy provided by the user, and modifying it to use the proper width for the specified dtype.

@qthequartermasterman qthequartermasterman self-assigned this Apr 26, 2024
@qthequartermasterman
Copy link
Owner Author

qthequartermasterman commented Apr 27, 2024

Hypothesis itself actually solves a similar problem when specifying width in st.floats. This is adapted from that solution:

        if width < 64:
            def downcast(x:float) -> float:
                """Downcast a float to a smaller width.

                This function is used to ensure that only floats that can be represented exactly are generated.

                Adapted from `hypothesis.strategies.numbers.floats`.

                Args:
                    x: The float to downcast.

                Returns:
                    The downcasted float.
                """
                try:
                    return float_of(x, width)
                except OverflowError:  # pragma: no cover
                    reject()
            elements = elements.map(downcast)

This seems to work great for fp16, fp32, and fp64. It fails spectacularly for bfloat16, because hypothesis does not support width=bfloat16 and the dynamic range of bfloat16 is not EXACTLY the same as fp32 (which I had used as a proxy). There are several fp32 values that are beyond the max for bfloat16. Hypothesis seems to find those values very quickly, and then takes eons in the shrinking phase finding other values at are inf in bfloat16, but not fp32.

@qthequartermasterman
Copy link
Owner Author

I accidentally pushed the partial solution to main: 8e9eb5d .

I meant to push to a branch and open a Draft PR. I really need to fix the branch protections in the repo. 😦

@qthequartermasterman
Copy link
Owner Author

I have pushed a fix that I am content with for now for bfloat16. It's not the cleanest solution in the world (filter out examples outside the min/max values of bfloat16, then let torch implicitly coerce values to the correct dtype), but hypothesis spends far less time generating valid examples than using (what in my opinion) the cleaner solution using struct packing/unpacking to truncate bits to turn float32 examples into bfloat16.

As referenced in HypothesisWorks/hypothesis#3959 (comment), there will likely be a future for hypothesis where it can natively handle generating bfloat16. When that day comes, I will likely refactor this to take advantage of those features.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
1 participant