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

[bug] Fix inconsistent results produced on different platforms #92

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

robomics
Copy link
Contributor

@robomics robomics commented Jan 13, 2025

Fix two bugs that resulted in different results being returned when running stripepy call on different platform (and potentially even when using different versions of numpy and pandas).

The first bug was caused by not forcing numpy and pandas to use stable sorting algorithms.
The second bug was due to minor difference in FP arithmetic on different architectures.

This first bug resulted in slightly different results produced on macOS and Linux (Windows and Linux produced consistent results).

The second bug resulted in different results when using different architectures (or even different CPUs with the same arch).

Fix a bug that resulted in different results being returned when running
stripepy call on different platform (and potentially even when using
different versions of numpy and pandas).

The bug was caused by not forcing numpy and pandas to use stable
sorting algorithms.
This resulted in slightly different results produced on macOS
and Linux (Windows and Linux produced consistent results).
@robomics robomics added the bug Something isn't working label Jan 13, 2025
@codecov-commenter
Copy link

codecov-commenter commented Jan 13, 2025

Codecov Report

Attention: Patch coverage is 96.00000% with 1 line in your changes missing coverage. Please review.

Project coverage is 85.40%. Comparing base (53c5e5c) to head (bf83f98).
Report is 15 commits behind head on main.

Files with missing lines Patch % Lines
src/stripepy/utils/common.py 92.85% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main      #92      +/-   ##
==========================================
+ Coverage   85.32%   85.40%   +0.07%     
==========================================
  Files          19       19              
  Lines        2481     2494      +13     
==========================================
+ Hits         2117     2130      +13     
  Misses        364      364              
Flag Coverage Δ
[tests integration python-3.10](https://app.codecov.io/gh/paulsengroup/StripePy/pull/92/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=paulsengroup)
[tests integration python-3.10
[tests integration python-3.11](https://app.codecov.io/gh/paulsengroup/StripePy/pull/92/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=paulsengroup)
[tests integration python-3.11
[tests integration python-3.12](https://app.codecov.io/gh/paulsengroup/StripePy/pull/92/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=paulsengroup)
[tests integration python-3.12
[tests integration python-3.13](https://app.codecov.io/gh/paulsengroup/StripePy/pull/92/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=paulsengroup)
[tests integration python-3.13
[tests integration python-3.9](https://app.codecov.io/gh/paulsengroup/StripePy/pull/92/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=paulsengroup)
[tests integration python-3.9
[tests unit python-3.10](https://app.codecov.io/gh/paulsengroup/StripePy/pull/92/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=paulsengroup)
[tests unit python-3.10
[tests unit python-3.11](https://app.codecov.io/gh/paulsengroup/StripePy/pull/92/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=paulsengroup)
[tests unit python-3.11
[tests unit python-3.12](https://app.codecov.io/gh/paulsengroup/StripePy/pull/92/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=paulsengroup)
[tests unit python-3.12
[tests unit python-3.13](https://app.codecov.io/gh/paulsengroup/StripePy/pull/92/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=paulsengroup)
[tests unit python-3.13
[tests unit python-3.9](https://app.codecov.io/gh/paulsengroup/StripePy/pull/92/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=paulsengroup)
[tests unit python-3.9

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@robomics robomics force-pushed the fix/portability branch 3 times, most recently from a2ed928 to c68b477 Compare January 13, 2025 16:02
Basically we truncate pseudodistribution values at the 10th decimal place
to avoid producing different results on different platforms/architectures
due to minor difference in FP values.
@robomics robomics marked this pull request as draft January 13, 2025 18:09
@robomics robomics marked this pull request as ready for review January 14, 2025 10:49
@robomics robomics requested a review from rea1991 January 14, 2025 11:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants