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

Make numerical and probability doctests ready for random seeds #29975

Closed
kliem opened this issue Jun 24, 2020 · 15 comments
Closed

Make numerical and probability doctests ready for random seeds #29975

kliem opened this issue Jun 24, 2020 · 15 comments

Comments

@kliem
Copy link
Contributor

kliem commented Jun 24, 2020

This ticket makes

sage -t --long --random-seed=n src/sage/numerical/
sage -t --long --random-seed=n src/sage/probabilty/

pass for different values n than just 0.

Depends on #29962

Component: doctest framework

Author: Jonathan Kliem

Branch/Commit: b6bef4b

Reviewer: Markus Wageringel

Issue created by migration from https://trac.sagemath.org/ticket/29975

@kliem kliem added this to the sage-9.2 milestone Jun 24, 2020
@kliem
Copy link
Contributor Author

kliem commented Jun 24, 2020

comment:1
sage -t --long --random-seed=151058820726654196682836430928254760259 src/sage/numerical/optimize.py  # 2 doctests failed
sage -t --long --random-seed=151058820726654196682836430928254760259 src/sage/probability/probability_distribution.pyx  # 18 doctests failed

@mkoeppe mkoeppe modified the milestones: sage-9.2, sage-9.3 Sep 5, 2020
@kliem
Copy link
Contributor Author

kliem commented Sep 18, 2020

Branch: public/29975

@kliem
Copy link
Contributor Author

kliem commented Sep 18, 2020

Commit: 4f65860

@kliem
Copy link
Contributor Author

kliem commented Sep 18, 2020

Author: Jonathan Kliem

@kliem
Copy link
Contributor Author

kliem commented Sep 18, 2020

New commits:

4453f7amake numerical ready for random seeds
4f65860make probability ready for random seeds

@mwageringel
Copy link

comment:4

Although this is unlikely to fail, the test should agree with the documentation, so please apply this change for the uniform distribution:

     Uniform distribution on the interval ``[a, b]``::
 
         sage: a = 0
         sage: b = 2
         sage: T = RealDistribution('uniform', [a, b])
-        sage: a <= T.get_random_element() < b
+        sage: a <= T.get_random_element() <= b
         True

For the Pareto distribution, you could also add a test that s >= b. Similarly, the Rayleigh, Lognormal, F, Chisquared and Weibull distributions are ≥ 0, and the Beta distribution lives on [0,1].

Finally, I think this test should document the expected outcome, for clarity:

         sage: [1.0*x/nr_samples for x in counts]  # abs tol 1e-1
-        [0.304200000000000, 0.397300000000000, 0.298500000000000]
+        [0.3, 0.4, 0.3]

I hope it is sufficiently unlikely that this test fails, but it is not impossible.

@mwageringel
Copy link

Reviewer: Markus Wageringel

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Sep 21, 2020

Changed commit from 4f65860 to c79005a

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Sep 21, 2020

Branch pushed to git repo; I updated commit sha1. New commits:

c79005amore precices doctests

@kliem
Copy link
Contributor Author

kliem commented Sep 21, 2020

comment:6

Thanks for improving the doctests.

Replying to @mwageringel:

Finally, I think this test should document the expected outcome, for clarity:

         sage: [1.0*x/nr_samples for x in counts]  # abs tol 1e-1
-        [0.304200000000000, 0.397300000000000, 0.298500000000000]
+        [0.3, 0.4, 0.3]

I hope it is sufficiently unlikely that this test fails, but it is not impossible.

I even modified it down to 3e-2. I tested it and the maximal difference is 0.0192 in 10 000 runs. So I guess it is much less likely than 1 in 10 000 that this test will fail.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Sep 21, 2020

Changed commit from c79005a to b6bef4b

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Sep 21, 2020

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

505d502more precices doctests
b6bef4b29975: more fixes

@mwageringel
Copy link

comment:9

Thanks for the updates. The change to multigraphics seems unintentional – I have removed it from your commit. I have also fixed another doctest:

sage -t --long --warn-long 55.4 --random-seed=2001 src/sage/numerical/optimize.py
**********************************************************************
File "src/sage/numerical/optimize.py", line 264, in sage.numerical.optimize.find_local_minimum
Failed example:
    plot(f, (x,-2.5, 2)).ymin()
Expected:
    -2.1827...
Got:
    -2.182677572710766

You can set this to positive if you agree with my changes.

@kliem
Copy link
Contributor Author

kliem commented Sep 21, 2020

comment:10

Thank you. Yes, agreed.

@vbraun
Copy link
Member

vbraun commented Sep 23, 2020

Changed branch from public/29975 to b6bef4b

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants