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

Fix sorting and byte-order errors with numpy 1.25 #1670

Merged
merged 21 commits into from Jul 20, 2023

Conversation

mcflugen
Copy link
Member

@mcflugen mcflugen commented Jul 19, 2023

Description

This pull request fixes a set of errors that cropped up with a recent version of numpy (v1.25) and (maybe) netcdf.

There were two sets of errors. The first were some doctests that read a sample netcdf file that was, apparently, written as big-endian. With the new package versions, the arrays were read from the file with their byte order retained—regardless of the native byte order of the machine the tests were run on. To fix this, I've just modified the doctests to convert the arrays to native byteorder.

The second set of errors were due to a change in argsort on some architectures. Certain architectures now use a vectorized version of the quicksort algorithm that does does not guarantee deterministic results with argsort (of course the values are still sorted but, in the case of a tie, the indices are not always guaranteed to be in the one order). I've fixed this by using the kind="stable" keyword for argsort.

I've also fixed another related issue where a doctest (FlowAccumulator.link_order_upstream) produced an array whose ordering was also not deterministic (possibly due to the underlying sorting algorithm of the architecture). The new tests check that downstream links always appear before upstream links but the specific ordering of links at an equal level is not guaranteed.

Checklist - did you ...

  • Add a news fragment file entry if necessary?
  • Add / update tests if necessary?
  • Add new / update outdated documentation?
  • All tests have passed?
  • Formatted code with black?
  • Removed lint reported by flake8?
  • Sucessful documentation built? (if documentation added or modified)

@mdpiper
Copy link
Member

mdpiper commented Jul 19, 2023

@mcflugen Wow, these are some nasty little bugs. Thank you for fixing them.

@gregtucker
Copy link
Contributor

🐛 🔥

@mcflugen
Copy link
Member Author

mcflugen commented Jul 19, 2023

It appears the failures in the notebook tests are a result of a new version of matplotlib (v3.7.2). If I limit matplotlib to less than 3.7.2, which I've done in this pull request, the failures go away. Note that the notebook tests only fail when they are run through the CI (I assume it has something to do with them being run on a headless server).

I'll try to fix this matplotlib issue as a separate pull request.

@mcflugen mcflugen merged commit 46dd4ed into master Jul 20, 2023
40 checks passed
@mcflugen mcflugen deleted the mcflugen/use-stable-argsort branch July 20, 2023 17:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants