Skip to content

Pin SciPy to >=1.7.3,<1.8.0 #5450

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

Merged
merged 2 commits into from
Feb 11, 2022

Conversation

michaelosthege
Copy link
Member

Also aligns the SciPy version pins across the testing environments for different Python versions.

See tracking issue #5448.

@michaelosthege michaelosthege added v3 installation issues about dependencies or installation labels Feb 6, 2022
@michaelosthege michaelosthege added this to the v3.11.5 (vNext) milestone Feb 6, 2022
@michaelosthege michaelosthege changed the title Pin SciPy to >=1.2.0,<1.8.0 Pin SciPy to >=1.2.0,<1.8.0 (v3) Feb 6, 2022
@michaelosthege michaelosthege changed the title Pin SciPy to >=1.2.0,<1.8.0 (v3) Pin SciPy to >=1.2.0,<1.8.0 Feb 6, 2022
@michaelosthege michaelosthege force-pushed the v3-pin-scipy-upper-limit branch 2 times, most recently from 3f61da0 to 4d21949 Compare February 6, 2022 16:05
@michaelosthege michaelosthege changed the title Pin SciPy to >=1.2.0,<1.8.0 Pin SciPy to >=1.7.3,<1.8.0 Feb 6, 2022
ericmjl
ericmjl previously approved these changes Feb 6, 2022
@canyon289
Copy link
Member

Rerunning failed jobs as failure was due to env setup flakiness not code changes

@ricardoV94
Copy link
Member

ricardoV94 commented Feb 6, 2022

Why the so restrictive lower version?

We use >1.4.1 in v4

@michaelosthege
Copy link
Member Author

There are shape problems and at least locally they went away with 1.7.3.
There were shape problems with 1.5.1 and at least locally they went away with 1.7.3, while I had numpy==1.22.2.

But the env creation doesn't work in all jobs, so there are also problems with NumPy.

The pytest workflow on this branch runs with Python 3.7, required numpy=1.15 and had conflics in env setup ❌
The arviz workflow on this branch runs with Python 3.8, required numpy>=1.15 and ran with numpy==1.22.2
The windows workflow on this branch runs with Python 3.9, required numpy>=1.15 and ran with numpy==1.21.5

Our CI envs have different pins for the numpy versions, but I don't understand why we pinned numpy=1.15 only for Python 3.7.
I'll try scipy>1.4.1,<1.8.0 and numpy>=1.15,<1.22.2 for all.

Version ranges for NumPy and SciPy were aligned between all CI envs.
Since Theano-PyMC is incompatible with the new SciPy 1.8.0,
this upper limit will most likely stay for all `v3` versions.

See tracking issue pymc-devs#5448.
twiecki
twiecki previously approved these changes Feb 7, 2022
@michaelosthege
Copy link
Member Author

That one failing test (https://github.com/pymc-devs/pymc/runs/5085570253?check_suite_focus=true#step:7:491) isn't just flaky and needs to be fixed before merging. There's no use in having a broken CI on the v3 branch.

This is a regression, likely caused by an updated NumPy/SciPy dependency.
Also see pymc-devs#5450
@michaelosthege michaelosthege merged commit b5205bb into pymc-devs:v3 Feb 11, 2022
@michaelosthege michaelosthege deleted the v3-pin-scipy-upper-limit branch February 11, 2022 12:15
@burnpanck
Copy link
Contributor

Why do you pin upper limits to versions in the dependency specification? Please do not do that just for the benefit of uniform versions in CI.

Upper version pinning should only be done when there is a known incompatibility, until compatibility has been re-established. If the dependency uses any sane version numbering system (and both numpy and scipy do!), then minor version number increases should be API backward compatible. Therefore, the expectation should be that minor version increases should be compatible. If they aren't, that's a bug on either end, which should be fixed (and a temporary solution may be a pin on the upper version). But unless there is, upper versions should not be pinned (except potentially before the next major version).

If you just want uniform versions in CI, explicitly select those versions in the CI environments. While the CI might test for an exact specific patch version such as numpy 1.22.1, that's still no reason to pin <1.22.2. It's not like the CI is testing every single patch version between 1.15 and 1.22.1. If Testing 1.22.1 does not give you the confidence that pymc3 will work on 1.22.2, why does testing 1.15 give you confidence that it works on 1.16.1?

@michaelosthege
Copy link
Member Author

@burnpanck this upper version pin was because of a known incompatibility and has since been removed after the incompatibility with SciPy 1.8.0 was fixed.

See #5448 for details.

We try to keep our CI environments free of exact pins and let mamba resolve which versions to use exacly.
But sometimes (like happened here) there are good reasons to temporarily pin upper versions.

@burnpanck
Copy link
Contributor

Ah, I hadn't found any reference to an issue with numpy. On a fresh install on my machine that pin was still active, which led me to jump to conclusions too quickly - sorry for that. I didn't realise I was installing an old version with 3.11.5, as I hadn't taken note of the name change with the move to v4.

Maybe it would make sense to at some point release an explicitly dead version 3.x once that branch is not being maintained anymore, so that downstream dependencies who did not take note of the name change either will fail loudly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
installation issues about dependencies or installation v3
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants