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.

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

TST: flaky TestSOSFreqz::test_fs_param #18737

Merged
merged 1 commit into from Jun 25, 2023
Merged

TST: flaky TestSOSFreqz::test_fs_param #18737

merged 1 commit into from Jun 25, 2023

Conversation

tylerjereddy
Copy link
Contributor

  • deals with the 1 of the 2 Apple silicon test failures in TST, MAINT: some tests blocking 1.11.0 on MacOS ARM64 with NumPy 1.25.0 #18732

  • I was able to reproduce locally, but because it was flaky I needed pytest-repeat for
    consistent failure:
    CC=clang CXX=clang++ FC=gfortran-12 python dev.py test -t scipy/signal/tests/test_filter_design.py::TestSOSFreqz::test_fs_param -- --count=200

  • I verified that with the provided patch/atol bump, we consistently pass even with 2000 repeats

[skip circle]

@tylerjereddy
TST: flaky TestSOSFreqz::test_fs_param
e3d6041 
* deals with the 1 of the 2 Apple silicon
test failures in scipygh-18732

* I was able to reproduce locally, but because
it was flaky I needed `pytest-repeat` for
consistent failure:
`CC=clang CXX=clang++ FC=gfortran-12 python dev.py test -t scipy/signal/tests/test_filter_design.py::TestSOSFreqz::test_fs_param -- --count=200`

* I verified that with the provided patch/`atol` bump,
we consistently pass even with `2000` repeats

[skip circle]
@tylerjereddy tylerjereddy added scipy.signal backport-candidate This fix should be ported by a maintainer to previous SciPy versions. labels Jun 24, 2023
@tylerjereddy tylerjereddy added this to the 1.12.0 milestone Jun 24, 2023
@tylerjereddy tylerjereddy mentioned this pull request Jun 24, 2023
Closed
ev-br
ev-br approved these changes Jun 25, 2023
View reviewed changes
Copy link
Member

@ev-br ev-br left a comment

Choose a reason for hiding this comment

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

LGTM. The assert_allclose default of atol=0 is a tripping point relatively often, so adding a non-zero atol is very reasonable. I'm a bit surprised that atol of 1e-27 is enough but if it is, it is. We can always bump it up if needs be.

@ev-br ev-br merged commit 283fc06 into scipy:main Jun 25, 2023
21 of 24 checks passed
@tylerjereddy tylerjereddy deleted the treddy_issue_18732_part1 branch June 25, 2023 13:52
@tylerjereddy tylerjereddy modified the milestones: 1.12.0, 1.11.0 Jun 25, 2023
@tylerjereddy tylerjereddy mentioned this pull request Jun 25, 2023
Merged
@tylerjereddy tylerjereddy removed the backport-candidate This fix should be ported by a maintainer to previous SciPy versions. label Jun 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ev-br ev-br ev-br approved these changes

Assignees
No one assigned
Labels
scipy.signal
Projects
None yet
Milestone
1.11.0
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@tylerjereddy @ev-br