Skip to content

MAINT: stats: adjust tolerance of failing TestTruncnorm #15916

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 2 commits into from
Apr 1, 2022
Merged

MAINT: stats: adjust tolerance of failing TestTruncnorm #15916

merged 2 commits into from
Apr 1, 2022

Conversation

mdhaber
Copy link
Contributor

@mdhaber mdhaber commented Apr 1, 2022 •
edited
Loading

Reference issue

gh-15871 (might close, might not)

What does this implement/fix?

scipy.stats.tests.test_distributions::TestTruncnorm::test_moments is failing in main on one of the CI jobs. This adjusts tolerances slightly so that this failure does not distract from failures specific to other PRs. Perhaps gh-15871 should remain open if the failure is truly considered a defect. If failure indicates that enhancement is decided, gh-15871 can be closed and linked to one of hte other issues about truncnorm.

Additional information

Instead, we could (temporarily) xfail the test to avoid suggesting that rtol=1e-5 is acceptable.

@mdhaber mdhaber added scipy.stats maintenance Items related to regular maintenance tasks labels Apr 1, 2022
@mdhaber mdhaber mentioned this pull request Apr 1, 2022
Closed
@tylerjereddy tylerjereddy added this to the 1.9.0 milestone Apr 1, 2022
tylerjereddy
tylerjereddy approved these changes Apr 1, 2022
View reviewed changes
Copy link
Contributor

@tylerjereddy tylerjereddy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds like a sensible plan--that test harness is pretty confined anyway based on git grep -E -i -n '_test_moments_one_range', and there is already a xfail_on_32bit in use here.

Hard to say if using a broader xfail vs. adjusting the tolerance is the right call, but I suspect we've gone both ways in similar situations in the past and we have a detailed issue from Warren that can remain open.

@tylerjereddy tylerjereddy merged commit f1a934b into scipy:main Apr 1, 2022
@mdhaber
Copy link
Contributor Author

mdhaber commented Apr 2, 2022

Thanks @tylerjereddy. I chose to go with the tolerance because although passing with tight tolerances on all platforms would be great, I don't see the failure as a defect. The test is know to be challenging in the tails and for higher moments, and that's where we're seeing the failure. truncnorm has already been rewritten once to improve it's accuracy in the tails (relative to a very generic implementation), and it achieved that. It would be great if the accuracy could be improved further, but I'd see that as an enhancement.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tylerjereddy tylerjereddy tylerjereddy approved these changes

Assignees
No one assigned
Labels
maintenance Items related to regular maintenance tasks scipy.stats
Projects
None yet
Milestone
1.9.0
Development

Successfully merging this pull request may close these issues.
2 participants
@mdhaber @tylerjereddy