-
Notifications
You must be signed in to change notification settings - Fork 21
Fix numpy>=2 compatibility in cdoTimeAverager and frepytoolsTimeAverager #721
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
Conversation
- Add explicit float64 dtype casting for time_bnds arrays - Replace Python sum() with np.sum() with explicit dtype parameter - Ensures consistent numerical results across numpy 1.x and 2.x - Addresses issue #711 regarding CDO producing different answers with numpy>=2 NumPy 2.0 changed type promotion rules (NEP 50), which can cause subtle numerical differences. This fix ensures consistent dtype handling by: 1. Explicitly casting time_bnds to float64 after reading from NetCDF 2. Using np.sum() with dtype=np.float64 instead of Python's sum() All CDO-related tests pass with these changes. Co-authored-by: ilaflott <6273252+ilaflott@users.noreply.github.com>
- Store result of np.moveaxis to avoid redundant computation - Fix trailing whitespace issue - Improves performance by eliminating duplicate transpose operation Co-authored-by: ilaflott <6273252+ilaflott@users.noreply.github.com>
- Remove unnecessary [:].copy() calls on array slices - Subtraction operation already creates a new array - Further improves performance while maintaining correctness Co-authored-by: ilaflott <6273252+ilaflott@users.noreply.github.com>
Remove 'main' branch trigger from pull request events.
|
- Add explicit float64 dtype casting for time_bnds arrays - Replace Python sum() with numpy.sum() with explicit dtype parameter - Optimize moveaxis operation to avoid redundant computation - Ensures consistent numerical results across numpy 1.x and 2.x The frepytools answer was indeed changing with numpy>=2 due to the same type promotion issues as CDO. This fix ensures both CDO and frepytools produce consistent results. Fixes the test_compare_unwgt_fre_cli_to_unwgt_cdo test failure. Co-authored-by: ilaflott <6273252+ilaflott@users.noreply.github.com>
You were right! The I fixed
The test Fixed in commit 90f7803. |
- Consolidate multi-line comment to be more concise - Remove redundant comment - Code review feedback addressed Co-authored-by: ilaflott <6273252+ilaflott@users.noreply.github.com>
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## use-numpy-v2 #721 +/- ##
===============================================
Coverage ? 82.77%
===============================================
Files ? 68
Lines ? 4529
Branches ? 0
===============================================
Hits ? 3749
Misses ? 780
Partials ? 0
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
|
looks backward compatible with |
Describe your changes
NumPy 2.0 changed type promotion rules (NEP 50), causing both
cdoTimeAveragerandfrepytoolsTimeAveragerto produce different numerical results than with numpy 1.x. The issue stems from implicit type conversions during weight calculations for time-averaged data.Changes to cdoTimeAverager.py:
time_bndsarrays tofloat64after reading from NetCDF to bypass NumPy 2.0's changed promotion behaviorsum()withnp.sum(dtype=np.float64)for consistent accumulation across numpy versionsnp.moveaxis()twice[:].copy()operations since subtraction already creates new arraysChanges to frepytoolsTimeAverager.py:
time_bndsarrays tofloat64to ensure consistent precisionsum()withnumpy.sum(dtype=numpy.float64)in weight calculationsum()withnumpy.sum(dtype=numpy.float64)in both weighted and unweighted calculation loopsImpact:
The fix ensures both CDO and frepytools produce consistent, identical results across numpy 1.x and 2.x. The critical test
test_compare_unwgt_fre_cli_to_unwgt_cdonow passes with 0.0 differences (previously had 37,110 non-zero differences at ~1e-10 level).Issue ticket number and link (if applicable)
Addresses #711
Checklist before requesting a review
✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.