Repository navigation
Adding property-based tests for Astropy using Hypothesis #9017
Description
At SciPy 2019, I did a talk and tutorial on Hypothesis, and at the sprints I was encouraged to open an issue or PR showing how it could be applied to Astropy. So here it is!
(and for SciPy 2020 I wrote a paper on property testing for science with principles, examples, and advice)
Traditional unit tests involve checking that for some specific (usually simple) input, you get the expected output. With Hypothesis, your tests instead check that some property is true for any valid input - you describe the domain, and it generates examples. (See the docs for details, but we spend a lot of time making tests non-flaky too)
For example, the following test will demonstrate a variety of ways that floating-point imprecision can ruin your day when working with units. This is unlikely to happen in practice, but careful use of Fraction could prevent it entirely...
from hypothesis import given, strategies as st
import astropy.units as u
UNITS = sorted(u.get_current_unit_registry().all_units, key=repr)
@given(st.lists(st.sampled_from(UNITS), min_size=3))
def test_intermediate_decomposition_is_no_op(lst):
decomp = compound = u.dimensionless_unscaled
for unit in lst:
decomp = (decomp * unit).decompose()
compound *= unit
assert decomp == compound.decompose(), (decomp, compound)Other properties that were suggested:
- Check that round-tripping from image to sky coordinates and back is lossless for distortion-free mappings, and otherwise always below 10^-5 px.
- Take a moment in time, round-trip it through various frames, check it hasn't changed or lost precision. See WIP: Add hypothesis testing for Time precision #9532 and Set up Hypothesis and add property-based tests for
Timeprecision #10373. - Testing that IO routines losslessly round-trip data that they are expected to handle
- Check that any optimised routines calculate the same result as unoptimised, within tolerances
- Check broadcasting behaviour of custom
ufuncs withmutually_broadcastable_shapes()
pytest-astropy pulls in Hypothesis automatically now, and Astropy has everything configured, so it's just a matter of writing some more tests! While I'm not an astrophysicist, I remain willing to review and otherwise help out with any property-based testing PRs for open science projects - just ping me 😁
Activity
pllim commentedon Jul 18, 2019
Hello and thank you for all these very useful and practical details.
I heard a lot of good things on Hypothesis at PyCon 2019 too. However, I do worry about how much longer CI would take with this switch. I think Hypothesis is great for regression tests in a nightly/weekly cron, but I am not 100% convinced for a CI that runs for every push event. Please enlighten me. Thanks again!
Zac-HD commentedon Jul 18, 2019
Thanks @pllim! There are a couple of responses to performance concerns:
100to e.g. 10 (or even lower), but left at the default or even increased for scheduled runs - you could even have a separate profile for local development. This would be set from aconftest.pyor similar, based on the detected environment.phasessetting, and rely on passing explicit examples instead. Or skip hypothesis tests withpytest -m "not hypothesis", as we automatically apply that marker.So there are lots of options to manage the performance impact, but IMO to apply them before seeing any slowdown would be premature optimisation 😄
mhvk commentedon Jul 18, 2019
@Zac-HD - thanks very much indeed. While you were answering @pllim's query (I share her desire to minimize testing time/power consumption), I had been looking at your documentation and had not only found the nice numpy array plugin but also the nice run-examples only option (so kudos to your documentation!). The latter might be appropriate for existing tests in newly opened PRs, leaving the random trials for daily-cron (and lots of trials for tests in the module the PR affects, especially new ones).
Anyway, agreed that too much worry about that is premature optimization!
Another practical question: for CIs, a database wit previous failures seems a bit less practical. Do you somehow still use it, or just add explicit examples for bugs uncovered? (Which can of course happen somewhat randomly...)
p.s. Did the example above uncover problems? (I'm the units maintainer...)
mhvk commentedon Jul 18, 2019
cc @bsipocz, @astrofrog
Zac-HD commentedon Jul 18, 2019
Aw, thanks 😊
My experience with Hypothesis tests in PRs is that it's actually very rare to find a bug for the first time when making an unrelated change, and less rare to discover an unexpected effect of the new change. We had one or two cases where a very rare edge cases eluded the tests for a week or so; but our current protocol is that if we suspect that's possible the PR developer will turn up the
max_examplessetting to 100K for a local run - if it's going to happen at all that will find it!Re example-database-in-CI: I'd suggest adding
.hypothesisto the list of directories cached by CI (or equivalently configuring the database setting to use such a directory). This database is basically designed for developer experience though, so for each bug found I'd capture it in an explicit@example(...)or traditional regression test to make sure it doesn't come back.And yep! The "unit test" (he he he) can find float overflow, underflow, and imprecision - potentially all in a single run. Whether this is a problem I think basically depends on your tradeoff between performance (floats are faster than Fractions) and precision (Fractions are exact, and unlike floats preserve that through scaling) - most of the examples it finds are combinations of six or more units which are probably (?) unlikely to arise in practice.
mhvk commentedon Jul 18, 2019
Thanks again, I now installed it locally at least...
Zac-HD commentedon Jul 18, 2019
@mhvk - I hope master...Zac-HD:add-hypothesis is illustrative, at least 😄
astrofrog commentedon Jul 19, 2019
I already use hypothesis in other packages and it has helped me find so many bugs! So I'm in favor of using it for some cases where it would make sense in the core package 👍
Regarding caching
.hypothesis, I think we should go with @Zac-HD's suggestion of instead capturing failing examples explicitly using@example, as any kind of caching on CI often leads to confusion when trying to reproduce failures locally.Zac-HD commentedon Jul 19, 2019
Just to clarify, I'd set up caching and capture failures as regression tests.
As to how you reproduce failures locally, Hypothesis will print instructions - as an explicit input where possible, or as a seed argument or decorator if not. That happens regardless of caching, as "user new to Hypothesis needs to reproduce a bug from CI" is one of our core use-cases 🙂
Zac-HD commentedon Sep 6, 2019
Ping @Juanlu001 from EuroScipy 😄
aarchiba commentedon Oct 6, 2019
Let me recommend hypothesis by pointing out bug #9328 which I found using hypothesis on PINT. It is excellent at uncovering numerical accuracy issues; see nanograv/PINT#549 for a beginner's attempt at hypothesizing (?) calculations. The runtime of the hypothesis tests is not noticeable in realtime, there was no need to cache the database, and troublesome examples are recorded in the source code. There is considerable repetition in the examples, but that is because my tests aren't very orthogonal.
astrofrog commentedon Oct 6, 2019
I think this is something that would make sense to discuss at the in-person coordination meeting in December, so I'll add it to the agenda.
aarchiba commentedon Oct 6, 2019
Let me add that if you understand where your corner cases are likely to occur you can encourage the example generator to focus on them. For example, I built a leap_sec_day_mjd() strategy to check days with leap seconds, and a normal_mjd() strategy to check days without - but normal_mjd() chooses between days-adjacent-to-leap-second-days and any-old-day-that-isn't-a-leap-second-day. And reasonable_mjd() chooses between those two, so leap-second-days and their neighbours get tried as part of the exploration process. I have found a few cases that still pop up after only a few hundred examples, but it's easy to propagate those to related tests once found.
I also found that I got more useful debugging when I used reasonable_mjds() to generate my examples and specified my constraints in real terms (these two conversion methods should not differ by more than a nanosecond).
Zac-HD commentedon Dec 9, 2019
The 2019 coordination meeting held a Hypothesis unconference - notes here: https://docs.google.com/document/d/14x7B0ykjWPyh2kh2GcbZTQn8PGdizg5zL7t03zDw7uU
mhvk commentedon May 25, 2020
For those following this thread, we now have very nice precision tests for
Timeusing hypothesis (via #10373, thanks to @aarchiba and @Zac-HD) - see https://github.com/astropy/astropy/blob/master/astropy/time/tests/test_precision.pyThose can serve as a good (if perhaps rather complicated!) example of how to set things up; there is also some more general information in the documentation, at https://docs.astropy.org/en/latest/development/testguide.html#property-based-tests.
astrofrog commentedon May 27, 2020
Just a thought - we might also want to provide strategies for other packages based on astropy, for example to generate random quantities, or random tables and so on.
mhvk commentedon May 27, 2020
Indeed, one could think of using our
testsmodule for that (either that ofastropyitself or, perhaps more logically, of each module), somewhat in analogy withnp.testing- this could also hold things likeassert_quantity_allcloseand standard fixtures. Indeed, I've seen in our own tests that we've started to import handy features from other tests; e.g.,astropy/astropy/table/tests/test_operations.py
Lines 21 to 22 in 0a46d33
Zac-HD commentedon May 27, 2020
Sounds great! I'd suggest 'dogfooding' them by using the strategies internally as a valuable way to check that the design actually makes sense, but something like an
astropy.testing.strategiesmodule would be awesome 😄Recommended reading: Hypothesis' guides to API design and efficient shrinking for strategies. They go into considerably more detail than most users should ever find useful, but are hopefully helpful if you're going to write custom strategies for downstream users.
Zac-HD commentedon May 30, 2020
Hi all - I have a draft paper in the SciPy proceedings (scipy-conference/scipy_proceedings#549) about property-based testing for scientific code, which uses some examples from this work.
Feel free to take a look, and let me know if you spot any inaccuracies or have suggestions on how to cite or describe the astro parts. General feedback would also be welcome, of course - I'm aiming to make this useful for both maintainers and users of scientific libraries!
mhvk commentedon May 30, 2020
@Zac-HD - thanks for sharing! I enjoyed reading it (but have no useful comments)
Zac-HD commentedon May 30, 2020
Thanks @mhvk! "no comments" still tells me I didn't make any egregious mistakes about astropy 😉
[-]Adding property-based tests for Astropy[/-][+]Adding property-based tests for Astropy using Hypothesis[/+]pllim commentedon Jul 16, 2022
At Scipy 2022, I dug up this issue again. Have we covered all the stuff this issue brought up?
Zac-HD commentedon Jul 16, 2022
No, only the
timetests: https://cs.github.com/astropy/astropy?q=%40given%20language%3APythonHappy to help out with this if anyone wants to sprint on it! (at SciPy, or via virtual attendance at the coordination meeting)
pllim commentedon Jul 16, 2022
Might be a good sprint topic for the next Astropy coordination meeting. Will add to the suggestions. Thanks for the idea, @Zac-HD !