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

CI: bump numpy version to fix errors #202

Merged
merged 3 commits into from
Nov 14, 2023
Merged

Conversation

jakevdp
Copy link
Contributor

@jakevdp jakevdp commented Nov 13, 2023

The CI tests check for iinfo.dtype and finfo.dtype, which were added to NumPy in numpy/numpy#23881 and therefore require a newer NumPy version.

Since newer numpy versions are not compatible with Python 3.8, I also bumped the Python versions used by the CI.

@jakevdp
Copy link
Contributor Author

jakevdp commented Nov 13, 2023

It looks like this addresses all the dtype-related errors of earlier versions, but numpy still doesn't define newaxis within the array API namespace.

@rgommers
Copy link
Member

Thanks @jakevdp, this LGTM. There was still one failure for test_solve, so I added that to the skip list too - let's see if it turns green now.

I opened numpy/numpy#25146 to fix the newaxis and linalg.solve issues.

@jakevdp
Copy link
Contributor Author

jakevdp commented Nov 14, 2023

The solve failure is intermittent (you can see it doesn't appear in the CI run for the first commit), and is related to the test framework. In my experience there are several dozen tests that have itermittent failures here. IMO it would be better to use the hypothesis derandomize setting rather than disabling every test that has an intermittent failure – what do you think?

@rgommers
Copy link
Member

+1 for derandomize in CI, yes. That is always best practice I'd say. Had several rounds of discussions with hypothesis folks on that when introducing Hypothesis to numpy/scipy CI, and I don't know why it's hard to convince them of that. It's basically always a terrible idea to have random/intermittent failures show up on PRs from contributors. Not using derandomize in CI only makes sense on a one-person or single-team project where everyone understands exactly what is happening.

The solve failure is intermittent

I don't understand the current FailedHealthCheck failure, but if the test suite gets past that it'll still be broken in numpy.array_api right now with undefined variable/function issues, see numpy/numpy#25146.

@honno
Copy link
Member

honno commented Nov 14, 2023

Brill thanks both, LGTM.

IMO it would be better to use the hypothesis derandomize setting rather than disabling every test that has an intermittent failure – what do you think?

+1 for derandomize in CI, yes. That is always best practice I'd say. Had several rounds of discussions with hypothesis folks on that when introducing Hypothesis to numpy/scipy CI, and I don't know why it's hard to convince them of that. It's basically always a terrible idea to have random/intermittent failures show up on PRs from contributors. Not using derandomize in CI only makes sense on a one-person or single-team project where everyone understands exactly what is happening.

I'll have a think, I've never used derandomize before heh.

The solve failure is intermittent

I don't understand the current FailedHealthCheck failure, but if the test suite gets past that it'll still be broken in numpy.array_api right now with undefined variable/function issues, see numpy/numpy#25146.

Personally I had just left the linalg stuff alone as I was hoping #101 would get merged, but I'll have a look as it hasn't moved in like a year lol.

@honno honno merged commit b6370dc into data-apis:master Nov 14, 2023
4 checks passed
@jakevdp jakevdp deleted the numpy-version branch November 14, 2023 16:43
@asmeurer
Copy link
Member

You use a lot of the benefits of hypothesis using derandomization. It could be useful for this test suite, but I'm not sure I would recommend it for numpy/scipy CI. According to the hypothesis docs if you have a derandomized run you should also have a separate randomized run that can catch bugs.

@rgommers
Copy link
Member

You need to do randomized runs either locally or separate from regular CI on PRs. It's like fuzz testing or benchmarking, it does not belong in regular CI. More like benchmarking in that it is also very slow and also looks for regressions. The linked hypothesis docs that suggest a separate nightly cron job are an improvement indeed, although I'd probably do it elsewhere.

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

4 participants