Add allow_subnormal kwarg to st.floats()

[honno]

Should resolve #3155. I think I've covered all the tricky areas in the newly introduced tests now, so my TODO is seemingly:

• Workout why test_allow_subnormal_defaults_correctly[allow_subnormal=True, min_value=-1, max_value=1] is failing
• Re-visit whatever I did in FixedBoundedFloatStrategy
• Clean and maybe refactor a few things
• Fix the weird auto-imports my editor did for me
• Clarify subnormal behaviour a bit in st.floats() docstirng
• Specific errors for flush-to-zero builds in extra.numpy.arrays() and extra.array_api.arrays()
• Allow allow_subnormal in extra.numpy.from_dtype() and extra.array_api.from_dtype()
• Fix and test kwarg toggling behaviour for out of range e.g. allow_subnormal=False is invalid when (0,1)
• Update RELEASE.rst

Making a PR now for CI + if anyone would be so kind to tell me if I'm on the right track 😅

[Zac-HD]

Yep, I think you're on the right track 🎉

Some small comments below. Note also that we'll want to have the same subnormal handling for numpy and the array_api extra.

[honno]

@Zac-HD thanks for all your great suggestions—all code comments should be resolved :)

RE the failing test_allow_subnormal_defaults_correctly test case—floats(-1, 1) does indeed default allow_subnormal correctly (to True), but the underlying FixedBoundedFloatStrategy(0, 1) doesn't generate subnormals in typical runs/TRY_HARDER runs (but it does certainly generate subnormals). Not ideal behaviour but I guess not considered a bug, definitely annoying to fix—probably worth resolving for #2907. I've left it xfailed for now.

There was a failing codemod test... TIL about codemods heh. I think my change to the test is okay but thought I'd mention it.

I'm mostly unavailable until next week so didn't want to sink time on adding NumPy/Array API arrays() error handling and from_dtype(allow_subnormal=...) support. You might want to merge this as-is (happy to resolve any tiny issues that come up), otherwise will return to it next week.

[honno]

I'm not sure yet why coverage is failing for lines 468 and 473 Ah I see I should be using tests/cover for these things.

[Zac-HD]

This is looking fantastic - thanks again @honno!

• I'd like to ship the array-related parts at the same time as the rest of it, and don't mind taking a few weeks longer to get it all right.
• Let me know if you'd like to try fixing the codemods, or want me to push a commit to your branch.
• For from_dtype() - as well as supporting allow_subnormal as a kwarg, I think we can address data-apis/array-api-tests#42 by passing False if we detect FTZ behaviour (which is cachable per module-and-bitwidth, and cheap with a one-element array). That way we can continue testing subnormals on libraries that represent them, and skip them for those which don't.
  (but still add the more specific error, for manually-passed element strategies)

[honno]

@Zac-HD I've added the subnormal inference and helpful error for extra.array_api now, if you'd like to take a look. I'll revisit it anyway tomorrow, and get the respective NumPy stuff done too.

See data-apis/array-api-tests#42 (comment) regarding the use of float32 over float64. I've learnt that different subnormal behaviour for different float dtypes is a thing, so will check ftz behaviour for each dtype.

[Zac-HD]

The helpful error looks great!

As you note we'll need a separate FTZ check for each module and size; I'd suggest having an @lru_cache()d function like ftz(xp, width) -> bool for this. Some comments below, but looking very nice overall and there's nothing new to do.

[honno]

As you note we'll need a separate FTZ check for each module and size; I'd suggest having an @lru_cache()d function like ftz(xp, width) -> bool for this. Some comments below, but looking very nice overall and there's nothing new to do.

Can't FTZ checks be done once in the make_strategies_namespace() body and carried over to its from_dtype wrapper? Also, I'm not sure if xp is always going to be cachable anywho i.e. they may not be modules or a SimpleNamespace etc..

[Zac-HD]

Yeah, once-per-size in make_strategies_namespace() is a better plan than mine 👍

[honno]

@Zac-HD RE allow_subnormal support for nps.from_dtype()... you think it's worth it? To clarify, nps.arrays() subnormal error message seems worth regardless.

With NumPy, I can ofc try building NumPy with ftz (15 min builds gets tiring fast when I inevitably screw something up and have to rebuild) and testing locally, but we'll probably want a ftz build of NumPy on CI too. With xps.from_dtype() we're probably going to test our allow_subnormal defaulting works on CI once 1) we support entry points #3085 (comment) 2) a lib like CuPy hopefully adopts it.

Theres a general maintenance burden of ftz inference and allow_subnormal defaulting too. All in all it might not be worth it seeing as there have been no related issues of it (thus far tbf). Thought I'd check before sinking time on it.

... or support allow_subnormal, but don't include ftz inference and respective allow_subnormal defaulting. The error message would mitigate much of the frustration for that rare FTZ NumPy user, and be much less of a maintenance burden as there'd be no inference/defaulting behaviour to test. I'll do this for now anywho.

[Zac-HD]

I think it's important to support allow_subnormal in from_dtype() - we're certainly going to want it eventually, and if it's in from the start then we avoid any awkward API gaps where subnormal support mostly worked for array libraries.

You make a good point about the maintainence/testing burden for automatic FTZ detection: let's defer that to a future PR.

[honno]

No idea why the win tests fail i.e. why are no subnormals generated with xps.from_dtype() when NumPy floats are not FTZ? The equivalent st.floats() tests work fine, hmm.

[Zac-HD]

No idea why the win tests fail i.e. why are no subnormals generated with xps.from_dtype() when NumPy floats are not FTZ? The equivalent st.floats() tests work fine, hmm.

Yeah, this is weird! I'm happy to add a skipif(WINDOWS) to them for now and we can open a follow-up issue after merging.

[Zac-HD]

Almost done! Thanks again @honno ... and remind me next time that we should chop your PRs into smaller pieces, they always end up growing into huge complicated things; I should remember this; and we can ship them in multiple easier stages instead.

[honno]

In #3156 (comment)

Let's make this a skip to save time, and to avoid strict-xfail making our tests fail if it passes.

IMO we should xfail instead of skip the tests that look for st.floats(-1, 1, allow_subnormal=True) generating subnormals. This is a consistent bug (the high max_examples of find_any attests to that) and it'd be nice to know when xpass occurs, so we can subsequently remove the marker, i.e. in #2907 if the internal float strategies are merged so NASTY_FLOAT sampling is always used. I think that makes sense? Feel free to ask me to drop the commit/remove it yourself.

[Zac-HD]

Xfailing doesn't work in this case, because the tests currently pass with very low probability. So in strict mode (which we use) they'd be flaky; in permissive mode they'd act like skips but take morw compute.

So we do need to skip, and I'll just remember them when the time comes.

[honno]

Oh right yeah flakiness 😅 Thanks for explaining

[Zac-HD]

I've done a full review, and think this is substantively complete! Thanks again and congratulations on yet another large PR 🎉

If you want to tackle the final move-some-things comments feel free; otherwise I'm happy to get them and then merge tomorrow.

[Zac-HD]

Suggested change

	:func:`hypothesis.extra.numpy.from_dtype` and :func:`xps.from_dtype` can also
	accept the ``allow_subnormal`` when inferring float strategies.
	:func:`xps.from_dtype` specifically will disable subnormals by default if the
	:func:`~hypothesis.extra.numpy.from_dtype` and :func:`xps.from_dtype` also
	accept the ``allow_subnormal`` argument, and :func:`xps.from_dtype` or
	:func:`xps.arrays` will disable subnormals by default if the

Missing word "argument", also affects xps.arrays(), minor sentence tweaks.

FYI you can click the readthedocs CI details to see a preview build, e.g. https://hypothesis--3156.org.readthedocs.build/en/3156/changes.html

[honno]

Suggestions make sense, updated accordingly... except for hypothesis.extra.numpy.from_dtype() I've explicitly specified it as nps.from_dtype(), to distinguish it from xps.from_dtype() (had purposely not used the tilde, think this is a happy compromise).

[honno]

@Zac-HD Thanks for all the effort you put pushing fixes and extensively reviewing for this PR! 😀