Utilizing Hypothesis in the ntheory/polys directory

[dianetc]

References to other Issues or PRs

In service to #20914, which seeks to introduce Hypothesis, a property based testing library, to the testing suite of Sympy.

Brief description of what is fixed or changed

This PR seeks to introduce Hypothesis to Sympy. Here various properties of certain functions are tested via Hypothesis. Namely, $\tau , \varphi $ (totient), and gcd. Certain properties of polynomial division are also tested.

The goal is for Hypothesis to be a testing dependency not a runtime dependency.

Other comments

Before this can pass, Hypothesis will need to be added to the CI.

Release Notes

NO ENTRY

[sympy-bot]

✅

Hi, I am the SymPy bot (v170). I'm here to help you write a release notes entry. Please read the guide on how to write release notes.

• No release notes entry will be added for this pull request.

Click here to see the pull request description that was parsed.

#### References to other Issues or PRs
In service to #20914, which seeks to introduce [Hypothesis](https://hypothesis.readthedocs.io/en/latest/quickstart.html), a property based testing library, to the testing suite of Sympy. 

#### Brief description of what is fixed or changed
This PR seeks to introduce Hypothesis to Sympy. Here various properties of certain functions are tested via Hypothesis. Namely,  $`\tau ,  \varphi `$ (totient), and gcd. Certain properties of polynomial division are also tested.

 The goal is for Hypothesis to be a testing dependency **not** a runtime dependency.

#### Other comments

Before this can pass, Hypothesis will need to be added to the CI. 

#### Release Notes
<!-- BEGIN RELEASE NOTES -->
NO ENTRY
<!-- END RELEASE NOTES -->

[sympy-bot]

🟠

Hi, I am the SymPy bot (v170). I've noticed that some of your commits add or delete files. Since this is sometimes done unintentionally, I wanted to alert you about it.

This is an experimental feature of SymPy Bot. If you have any feedback on it, please comment at sympy/sympy-bot#75.

The following commits add new files:

• fc7ba14:
  • sympy/ntheory/tests/test_tau_hypothesis.py
  • sympy/ntheory/tests/test_totient_hypothesis.py
  • sympy/polys/tests/test_gcd_hypothesis.py
  • sympy/polys/tests/test_poly_hypothesis.py

The following commits delete files:

• d3bd0ff:
  • sympy/ntheory/tests/test_totient_hypothesis.py
  • sympy/polys/tests/test_gcd_hypothesis.py

If these files were added/deleted on purpose, you can ignore this message.

[asmeurer]

CC @honno

[asmeurer]

Hi @dianetc thanks for this pull request! I left a few comments on the tests. I'd also like for @honno to review this as well.

We need to make sure we add hypothesis to the CI here before this can be merged (by installing hypothesis in the appropriate place in the .github/ workflow file(s)). We should also update the contributor guide to mention hypothesis. Anywhere that says to install "pytest" should be updated to also say to install hypothesis. It would also be good to add something to https://github.com/sympy/sympy/blob/master/doc/src/contributing/new-contributors-guide/writing-tests.md and to https://github.com/sympy/sympy/blob/master/doc/src/contributing/dependencies.md about hypothesis.

Note to everyone that hypothesis will just be added as a testing dependency only, not a runtime dependency. I personally feel like it should be a required testing dependency, but if people feel strongly about it we can make it an optional dependency instead.

[dianetc]

Currently the CI fails because running node bin/test_pyodide.mjs --group=${{ matrix.group }} --splits=4 2>/dev/null] (found here) doesn't load up the hypothesis package.

Pyodide doesn't support Hypothesis , so it seems there may be two options (?):

1. add Hypothesis to the pyodide distribution then update test_pyodide.mjs accordingly.
2. add a run: pip install Hypothesis in workflow/runtests.yml .

I'm not too sure if option number 2 will work. Moreover, it seems to undermine the whole point of that test.

@asmeurer @honno

[asmeurer]

It looks like one of the tests failed with

_____________________________ test_tau_hypothesis ______________________________
sympy/ntheory/tests/test_tau_hypothesis.py:10: in test_tau_hypothesis
    def test_tau_hypothesis(n):
/opt/hostedtoolcache/Python/3.10.12/x64/lib/python3.10/site-packages/hypothesis/core.py:737: in test
    raise DeadlineExceeded(runtime, self.settings.deadline)
E   hypothesis.errors.DeadlineExceeded: Test took 500.26ms, which exceeds the deadline of 200.00ms

The above exception was the direct cause of the following exception:
sympy/ntheory/tests/test_tau_hypothesis.py:10: in test_tau_hypothesis
    def test_tau_hypothesis(n):
/opt/hostedtoolcache/Python/3.10.12/x64/lib/python3.10/site-packages/hypothesis/core.py:814: in execute_once
    raise Flaky(
E   hypothesis.errors.Flaky: Hypothesis test_tau_hypothesis(n=217) produces unreliable results: Falsified on the first call but did not on a subsequent one
E   Falsifying example: test_tau_hypothesis(
E       n=217,
E   )
E   Unreliable test timings! On an initial run, this test took 500.26ms, which exceeded the deadline of 200.00ms, but on a subsequent run it took 0.06 ms, which did not. If you expect this sort of variability in your test timings, consider turning deadlines off for this test by setting deadline=None.
E   
E   You can reproduce this example by temporarily adding @reproduce_failure('6.82.0', b'AG8GAAIA2A==') as a decorator on your test case

[asmeurer]

The reported flakiness could be investigated, but it's likely due to certain things being cached. That sort of thing is quite common in SymPy, unfortunately, meaning these timing flakiness checks are not going to be very helpful. Is there a way to disable timing flakiness checks without also disabling the deadline entirely?

[honno]

Is there a way to disable timing flakiness checks without also disabling the deadline entirely?

Don't think so 🙃

Note @dianetc you can apply deadline=None globally by putting something a Hypothesis settings profile in a conftest.py (i.e. sympy/conftest.py covers everything in this PR)

from hypothesis import settings

settings.register_profile("no_deadline", deadline=None)
settings.load_profile("no_deadline")

[github-actions]

Benchmark results from GitHub Actions

Lower numbers are good, higher numbers are bad. A ratio less than 1
means a speed up and greater than 1 means a slowdown. Green lines
beginning with + are slowdowns (the PR is slower then master or
master is slower than the previous release). Red lines beginning
with - are speedups.

Significantly changed benchmark results (PR vs master)

Significantly changed benchmark results (master vs previous release)

       before           after         ratio
     [8059df73]       [db5ca227]
     <sympy-1.12^0>                 
-      84.6±0.8ms       55.9±0.3ms     0.66  integrate.TimeIntegrationRisch02.time_doit(10)
-      84.3±0.9ms       54.9±0.4ms     0.65  integrate.TimeIntegrationRisch02.time_doit_risch(10)
-     2.11±0.01ms          631±2μs     0.30  polys.TimePREM_LinearDenseQuadraticGCD.time_op(3, 'sparse')
-     10.5±0.04ms      1.88±0.01ms     0.18  polys.TimePREM_LinearDenseQuadraticGCD.time_op(5, 'sparse')
-       363±0.6μs       78.9±0.5μs     0.22  polys.TimePREM_QuadraticNonMonicGCD.time_op(1, 'sparse')
-     4.78±0.01ms          351±1μs     0.07  polys.TimePREM_QuadraticNonMonicGCD.time_op(3, 'sparse')
-     10.6±0.03ms         1.06±0ms     0.10  polys.TimePREM_QuadraticNonMonicGCD.time_op(5, 'sparse')
-     6.25±0.02ms      3.85±0.01ms     0.62  polys.TimeSUBRESULTANTS_LinearDenseQuadraticGCD.time_op(2, 'sparse')
-     27.5±0.05ms      11.7±0.03ms     0.42  polys.TimeSUBRESULTANTS_LinearDenseQuadraticGCD.time_op(3, 'sparse')
-     6.72±0.02ms      1.13±0.01ms     0.17  polys.TimeSUBRESULTANTS_QuadraticNonMonicGCD.time_op(1, 'sparse')
-     15.7±0.03ms      8.99±0.02ms     0.57  polys.TimeSUBRESULTANTS_QuadraticNonMonicGCD.time_op(2, 'sparse')
-       206±0.3ms      68.9±0.09ms     0.33  polys.TimeSUBRESULTANTS_QuadraticNonMonicGCD.time_op(3, 'sparse')
-     6.39±0.01ms          511±2μs     0.08  polys.TimeSUBRESULTANTS_SparseGCDHighDegree.time_op(3, 'sparse')
-     27.4±0.03ms          816±2μs     0.03  polys.TimeSUBRESULTANTS_SparseGCDHighDegree.time_op(5, 'sparse')
-         591±2μs        202±0.7μs     0.34  polys.TimeSUBRESULTANTS_SparseNonMonicQuadratic.time_op(1, 'sparse')
-        6.38±0ms          207±1μs     0.03  polys.TimeSUBRESULTANTS_SparseNonMonicQuadratic.time_op(3, 'sparse')
-     16.7±0.06ms        209±0.6μs     0.01  polys.TimeSUBRESULTANTS_SparseNonMonicQuadratic.time_op(5, 'sparse')

Full benchmark results can be found as artifacts in GitHub Actions
(click on checks at the top of the PR).

[asmeurer]

It looks like the remaining things to do here are:

• Merge the test files together
• Make sure hypothesis is mentioned in the contributor documentation files

[asmeurer]

Going to merge this now. We can add more tests in later PRs.

In the future, we'll want to expand the hypothesis documentation in the tests docs, especially once we start adding custom strategies. For now, I'm hopeful that this is enough that other people can start playing around with hypothesis tests as well.

[oscarbenjamin]

It might be worth putting out an announcement somewhere about this change:

$ pytest sympy/polys/
ImportError while loading conftest '/home/oscar/current/active/sympy/sympy/conftest.py'.
sympy/conftest.py:10: in <module>
    from hypothesis import settings
E   ModuleNotFoundError: No module named 'hypothesis'

Also the guide here should be corrected and hypothesis made more prominent:
https://github.com/dianetc/sympy/blob/31b6a9400440cd2068d776f66a34cdc8254c38bc/doc/src/contributing/dependencies.md

Running the Tests The base SymPy tests do not require any additional
dependencies, however most of the above dependencies may be required for some
tests to run. Tests that depend on optional dependencies should be skipped when
they are not installed, either by using the sympy.testing.pytest.skip()
function or by setting skip = True to skip the entire test file. Optional
modules in tests and SymPy library code should be imported with
import_module().

pytest: Pytest is not a required dependency for the SymPy test suite. SymPy has
its own test runner, which can be accessed via the bin/test script in the SymPy
source directory or the {func}~.test function.

However, if you prefer to use pytest, you can use it to run the tests instead
of the SymPy test runner. Tests in SymPy should use the wrappers in
{mod}sympy.testing.pytest instead of using pytest functions directly.

Cloudpickle: The cloudpickle package can be used to more effectively pickle
SymPy objects than the built-in Python pickle. Some tests in
sympy.utilities.tests.test_pickling.py depend on cloudpickle to run. It is not
otherwise required for any SymPy function.

hypothesis: Hypothesis is a required dependency for the SymPy test suit.

[oscarbenjamin]

When I say that the guide should be corrected I don't just mean in relation to hypothesis. It is also outdated in its information about pytest although I'm not sure if pytest is now a hard dependency (CC @brocksam).

Also it is probably not worth mentioning cloudpickle at all here.

[asmeurer]

The point of that dependencies page is to list every optional dependency of SymPy, because there are so many of them it's useful to have that information gathered into one place. That's why cloudpickle is listed there. It really isn't supposed to be just about the important ones. The other contributor guide pages are really where we should be emphasizing that pytest and hypothesis are development dependencies.

[oscarbenjamin]

Cloudpickle is just an optional dependency like the rest in the bigger list of optional dependencies.

The section on running the tests should clearly state that both pytest and hypothesis are hard requirements to be able to run the tests (if that is true). Also since this page is for contributors the hard requirements just to be able to run the tests should be listed higher up by the hard runtime dependency mpmath rather than below the optional dependencies (most of which are often not needed at all).

[asmeurer]

I agree that the developer documentation should be updated to reflect pytest being a development dependency. It seems that was never done. There are some minor improvements in that area in #25468 and #25467 but we should do a more thorough update.