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

[MRG] fix: relax test for poisson drive stats #978

Merged

Conversation

asoplata
Copy link
Collaborator

This is a trivial fix for stochastic, difficult-to-reproduce test failure. Rarely, and seemingly only on Python 3.8 Unit Test runners, this test will fail due to the random Poisson mean interval exceeding the approx threshold. Here is an example case: https://github.com/asoplata/hnn-core/actions/runs/12952951618/job/36131478285#step:9:164 . I noticed the same thing happening on the first time I tried to run the tests for #971 ; however, since I told it to re-run the failed jobs, now I can't find any data on the previously-failed jobs. (It's possible that when you re-run failed jobs, maybe you can't access the old failed data using GitHub Actions?). As you can see in the above example, the test failed because the mean event interval was around 18.98, which is just outside of the threshold of 20 +/- 1.

This may have been a pre-existing issue that sometimes produced random failures in the tests, but only rarely and stochastically. By expanding the tolerance for this test, this should become far less likely to cause test failures in the future.

To be clear, scientifically, we're saying that the mean of our realized Poisson rate is within 1.5 ([Hz] I believe) of our expected value, instead of 1 [Hz], for testing purposes. We could also tighten the tolerance gap back to something like 1.1 if anyone else is uncomfortable with this change (@ntolley ?)

@asoplata asoplata added bug Something isn't working testing labels Jan 24, 2025
@asoplata asoplata requested a review from ntolley January 24, 2025 17:13
Copy link
Contributor

@ntolley ntolley left a comment

Choose a reason for hiding this comment

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

Fascinating edge case! Thanks @asoplata!

@ntolley ntolley merged commit 1ef293b into jonescompneurolab:master Jan 27, 2025
12 checks passed
@asoplata asoplata deleted the relax-float-poisson-drive-test branch January 28, 2025 15:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working testing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants