numpy 2.0.0rc migration #5790

h-vetinari · 2024-04-20T01:58:00Z

This follows conda-forge/conda-forge.github.io#1997 & discussion in core this week.

Previously...

There are a couple of problems, most of which stem from the existing zip between {numpy, python, python_impl}. Due to this zip, wanting to build for both numpy 1.x & numpy 2.0 means we have to duplicate all keys involved in the zip. I've been testing this on the scipy-feedstock, see conda-forge/scipy-feedstock@5c8599d.

However, the immediate problems that arise are:

channel_sources does not get populated correctly despite being update in the migrator (am I missing something...?)
migrations with operation: key_add can apparently only add one key, so - after the necessary duplication due to the zip - the changes in python312.yaml and pypy38.yaml end up dropping the existing 1.x builds
using additional_zip_keys: channel_sources (like for python 3.12.0rc) leads to RecursionError: maximum recursion depth exceeded in smithy (see conda-forge/scipy-feedstock@22d530f).

After discussion in that issue, it seems we're tending towards not dealing with duplicating all numpy builds into 1.x & 2.0rc1, but rather building everything against 2.0rc1 directly (problems with that outlined below).

However, one issue from the original approach still remains:

channel_sources does not get populated correctly despite being update in the migrator (am I missing something...?)

Despite setting channel_sources in the migrators unequivocally, it ends up getting ignored by conda-smithy. I have a local smithy fix that I'm testing in conda-forge/scipy-feedstock#274 together with the migrator state from this PR.

conda-forge-webservices · 2024-04-20T01:58:06Z

Hi! This is the friendly automated conda-forge-linting service.

I just wanted to let you know that I linted all conda-recipes in your PR (recipe) and found it was in an excellent condition.

h-vetinari · 2024-04-20T11:11:38Z

Argh, damn. If we set channel_sources unconditionally, we'll break on 3.8 (c.f. conda-forge/pythran-feedstock#84)

conda_libmamba_solver.conda_build_exceptions.ExplainedDependencyNeedsBuildingError: Unsatisfiable dependencies for platform win-64: {MatchSpec("numpy==1.22.4=py39h503c56f_0")}
Encountered problems while solving:
  - package numpy-1.22.4-py39h503c56f_0 is excluded by strict repo priority

h-vetinari · 2024-04-20T12:50:41Z

Argh, damn. If we set channel_sources unconditionally, we'll break on 3.8 (c.f. conda-forge/pythran-feedstock#84)

I cannot get this to work for pythran despite hacking around on conda-smithy. The combination of having several migrations use additional_zip_keys is not something the code is ready for AFAICT (because then it'll already have been added by the time we merge the second migration).

However we can't avoid additional_zip_keys, because otherwise there's no defined ordering between plain conda-forge (once, for py38) and conda-forge/label/numpy_dev,conda-forge (for everything else) and so the variant algebra fails to merge (or produces garbage).

The other issue is that AFAICT additional_zip_keys was only ever developed/tested for operation: key_add migrations. Here we're on a kind: version migration though. However, I haven't managed to figure this out in smithy.

Perhaps that difference can be exploited in a different way though - if we change the global pinning to zip in channel_sources, the rest could be made to work (it would stay on main for non-migrated feedstocks of course). But I'm not sure if we want to use such a big hammer.

h-vetinari · 2024-04-20T23:24:47Z

Another alternative of course would be to drop python 3.8, see #5013

h-vetinari · 2024-04-29T02:30:32Z

@conda-forge/core, we should come to a decision here what we want to do. AFAICT, we could either:

drop Python 3.8
zip channel_sources into python globally
use channel_priority: loose (if we can, we could restrict that to 3.8)
improve smithy to have working additional_zip_keys across multiple migrations

I doubt that 4. is going to materialize, 3. is IMO a way too big hammer, but 1. & 2. should work.

xhochy · 2024-04-29T13:25:18Z

will not work in a straight-forward way (with experience from the Python 3.12 migration). There are some packages (arrow-cpp ;) ) that build all Python versions in a single build. There, you will still get the solving error. You would need to do the same dance as we did with Python 3.12 and create a numpy-rc package that is in this channel and upload the numpy RC itself on main.

I feel like this is making a case for dropping Python 3.8. I'll comment my support over there.

isuruf · 2024-04-29T13:35:24Z

will not work in a straight-forward way (with experience from the Python 3.12 migration). There are some packages (arrow-cpp ;) ) that build all Python versions in a single build. There, you will still get the solving error. You would need to do the same dance as we did with Python 3.12 and create a numpy-rc package that is in this channel and upload the numpy RC itself on main.

This seems like a reasonable thing to do.

h-vetinari · 2024-04-30T04:55:29Z

You would need to do the same dance as we did with Python 3.12 and create a numpy-rc package that is in this channel and upload the numpy RC itself on main.

I had thought I had followed the python 3.12 rollout, but I'm not aware of what kind of -rc package you're talking about - could you point me to where this was produced or discussed? FWIW, the arrow PR for the 3.12 migration just did

 channel_sources:
 - conda-forge
+- conda-forge
+- conda-forge/label/python_rc,conda-forge
+- conda-forge
+- conda-forge
 zip_keys:
 - - python
   - numpy
+  - channel_sources

so I don't see the problem there. AFAICT, it would be harmless (i.e. no functional change) to change the global pinning to do

  channel_sources:
  - conda-forge
+ - conda-forge
+ - conda-forge
+ - conda-forge
 zip_keys:
 - - python
   - python_impl
   - numpy
+  - channel_sources

which would allow us to override channel_sources per python version in the migrator (and then be able to leave 3.8 untouched).

xhochy · 2024-04-30T07:29:44Z

See https://github.com/conda-forge/python-feedstock/blob/46e4991703e49cf75833699d42d6605147d31aa6/recipe/meta.yaml#L185-L187 and https://github.com/conda-forge/_python_rc-feedstock Only the -rc package was uploaded to the python_rc channel.

h-vetinari · 2024-04-30T09:11:02Z

OK, thanks for that refresher! I opened conda-forge/staged-recipes#26188 for creating _numpy_rc.

h-vetinari · 2024-05-01T07:30:13Z

The issues here should have been solved now AFAICT. conda-forge/conda-smithy#1911 could be solved without changes to smithy (see 0aeca11), and the question about how to deal with the channel_sources vs. strict channel priority vs. python 3.8 vs. smithy constraints has also been solved by following the same approach as for the python 3.12rc migration (see conda-forge/staged-recipes#26188 and conda-forge/numpy-feedstock#314).

Once we merge the numpy PR, we can also re-test this in conda-forge/pythran-feedstock#84, but AFAICT we're basically ready now.

This way it also ends up in the PR body, see https://github.com/regro/cf-scripts/blob/a14a4532d2618fc7721aa6ce959639b3c9769dae/conda_forge_tick/migrators/migration_yaml.py#L311

…f failures

recipe/migrations/pypy38.yaml

jakirkham

Thanks Axel! 🙏

Had a couple thoughts on the message to maintainers below

recipe/migrations/numpy2.yaml

jakirkham · 2024-05-02T22:45:49Z

To provide some context on some of the suggestions above, think the main issue is this is a lot of text for maintainers to read and this going to show up in a lot of feedstocks. Would like to see if we can find a balance between informative and overwhelming

Suppose one could say all of the words written currently need to be there and maybe that is true. Though suspect we can condense this a bit more. The more succinct it becomes the better chance that people will read and understand it

Made an attempt to do that, but of course there are many ways to accomplish this. Please feel free to make other changes that accomplish this

h-vetinari · 2024-05-02T23:21:51Z

The more succinct it becomes the better chance that people will read and understand it

I agree with the sentiment, though I do think the information is way more accessible to maintainers if it's already described in the PR itself, rather than having to go hunt around for it. Secondly, we're already providing a TL;DR if someone doesn't want to read the whole thing, so I don't really buy the "overwhelmed" bit. If maintainers don't read the PR and just click merge on a green CI, there's not much we can do, and should be fine. If they take 2min to read it, even better.

So while I agree we should strive for succinctness, we also shouldn't go to the point where the information provided becomes inaccurate. And I think the information provided does have a significant value add for those wanting to understand what's happening (which I wager is higher than those who are scared of by a few paragraphs of text).

h-vetinari · 2024-05-03T21:03:48Z

recipe/migrations/numpy2.yaml

+      * [ ] Match run-requirements for numpy (i.e. check upstream `pyproject.toml` or however the project specifies numpy compatibility)
+        * If upstream is not yet compatible with numpy 2.0, add `numpy <2` upper bound under `run:`.
+        * If upstream is already compatible with numpy 2.0, nothing else should be necessary in most cases.
+        * If upstream requires a minimum numpy version newer than 1.19, you can add `numpy >=x.y` under `run:`.


I'm not sure how much we want to talk about lower bounds, because this is often not necessarily correct upstream either. But I think we need to mention it

h-vetinari · 2024-05-03T21:06:06Z

@rgommers, regarding not mentioning NPY_TARGET_VERSION, I was wondering about the following scenario:
What happens if a project already uses features of the numpy C-API that are newer than 1.19? Does the build fail then? If it does, what is the right way to fix it?

h-vetinari · 2024-05-04T03:41:18Z

OK, I'm going to merge this to get the ball rolling. It's very easy to adapt the commit message to something else afterwards, while the migration is running - if you feel something can be improved or should be changed, please comment or open a PR! :)

jakirkham · 2024-05-04T04:29:56Z

Thanks Axel! 🙏

Appreciate you taking the time to revise the message a bit. Communication of this kind is always tricky and I'm sure we will learn from maintainers' feedback

Though agree it is better to get this moving than perfect the message. Thanks for doing that 🙂

h-vetinari · 2024-05-04T07:18:27Z

recipe/migrations/numpy2.yaml

+python:
+  - 3.8.* *_cpython
+  - 3.9.* *_cpython
+  - 3.10.* *_cpython
+  - 3.11.* *_cpython


So I just realized that having this bit in the migrator is a problem, because it considers all python-feedstocks as up for migration. I'll try to reduce that.

Fixed in 1aef67b (after testing that the rerender in scipy didn't change)

rgommers · 2024-05-04T08:28:24Z

@rgommers, regarding not mentioning NPY_TARGET_VERSION, I was wondering about the following scenario:
What happens if a project already uses features of the numpy C-API that are newer than 1.19? Does the build fail then? If it does, what is the right way to fix it?

Yes, the build will fail because the C API in question will not be exposed in the numpy headers. I would not expect this to ever happen in conda-forge feedstocks, because it won't be possible to build wheels for PyPI if there is such a problem in the sources of a package. The fix has to be at the package source level.

h-vetinari · 2024-05-04T09:29:18Z

I would not expect this to ever happen in conda-forge feedstocks, because it won't be possible to build wheels for PyPI if there is such a problem in the sources of a package

I don't understand how that's related to PyPI? If a package specifies >=1.24 and uses the respective newer C API features, that should be a normal use case?

rgommers · 2024-05-04T09:49:26Z

Then I misunderstood your question. If it uses the new C API features and sets >=1.24 and sets NPY_TARGET_VERSION, things will work. And there is nothing to do on the feedstock. If it fails to set NPY_TARGET_VERSION, the build will fail when it does its release to PyPI already, so that case shouldn't show up.

If that's still not clear, can you please make the question more precise?

h-vetinari · 2024-05-04T10:13:16Z

and sets NPY_TARGET_VERSION

That's what my question boiled down to. It's possible that packages/feedstocks are specifying numpy >1.2x and justifiably using the newer C API features. In a pre-numpy-2.0 world that worked, but for such cases that are already using newer C API features, yet not yet setting NUMPY_TARGET_VERSION upstream, we'd have to do that on the feedstock? Or should we just fail and wait for a numpy 2.0 compatible upstream version?

rgommers · 2024-05-04T10:20:33Z

but for such cases that are already using newer C API features, yet not yet setting NUMPY_TARGET_VERSION upstream,

Again, this is not possible. The build will fail, since the newer C API features are not exposed unless NUMPY_TARGET_VERSION is set correctly in the build config files of the package.

h-vetinari · 2024-05-04T10:26:04Z

How is that not possible in a pre-2.0 world? If numpy 1.24 is available at build time, there's no need for setting the macro AFAICT.

rgommers · 2024-05-04T10:41:01Z

The whole mechanism was only introduced in numpy 1.25, so for numpy 1.24 things are as they always were. That's completely unrelated to either NPY_TARGET_VERSION or the 2.0 migration I think, so I really don't understand the problem or why it's relevant on a discussion about 2.0 migration.

Also, aside from it being unrelated: such a build config would have started failing in the package's CI as soon as numpy 1.25 became available, so it would have been fixed a long time ago.

It would be nice at this point to either see a real-world issue (because this has been entirely hypothetical for weeks now) or a very very clear description of the problem.

h-vetinari · 2024-05-04T10:53:51Z

I'm thinking / concerned about the following scenario:

Package already uses newer C API features (say 1.22, which is our current pinning)
Feedstock pins host version to something before numpy 1.25 at build time (compare yet again the current default of 1.22 in conda-forge)
Latest released version is not yet ready for 2.0

In that case, if we now update to build against numpy 2.0 but for the 1.19 C API, things would fail (AFAICT). I'm asking what steps we should recommend to such feedstocks to solve eventual build issues due to the above, in the face of the upcoming migration PRs to such feedstocks.

rgommers · 2024-05-04T12:07:20Z

So I'd say:

If the package is not yet ready for 2.0, don't try to unilaterally patch it in a feedstock. Hold the migration there and file an issue upstream. If it's fixed there, take over the fix.
This really is highly unlikely to occur in the real world. Because the package would be broken in a regular build against numpy 1.25/1.26, so distro maintainers would have filed issues against the package last year already.

I propose we close the case here, and only revisit it if and when something actually materializes. There's no point speculating here.

xhochy · 2024-05-04T15:40:49Z

recipe/migrations/python312.yaml

@@ -33,6 +33,8 @@ python:
  - 3.12.* *_cpython
 # additional entries to add for zip_keys
 numpy:
-  - 1.26
+  - 2.0


@h-vetinari This will break all feedstocks with Python 3.12. Please revert.

Cc @hmaarrfk

How so? I've tested this with a bunch of feedstocks building for 3.12 successfully.

I'm happy to revert of course, but can someone describe (or link to) the breakage?

Ah nevermind, I understand what you mean. It changes py312 builds even before the migrator comes by. 😑

That's an interesting wrinkle. I'm not sure how we can then do the numpy migration for 3.12...

Done: 09f6263

I've continued the discussion how to fix this in conda-forge/conda-forge.github.io#1997

h-vetinari · 2024-05-05T22:10:49Z

The comment formatting here gets completely busted due to this...

h-vetinari · 2024-05-05T22:24:11Z

The comment formatting here gets completely busted due to this...

Fix in regro/cf-scripts#2538

numpy 2.0.0rc migration

d55958b

h-vetinari mentioned this pull request Apr 20, 2024

Rebuild for numpy 2.0 conda-forge/scipy-feedstock#274

Merged

jakirkham mentioned this pull request Apr 20, 2024

NumPy 2 bringup conda-forge/conda-forge.github.io#1997

Open

10 tasks

build everything against numpy 2.0.0rc1 right away

93399ad

jakirkham mentioned this pull request Apr 25, 2024

Updates to work with numpy 2.0 holoviz/holoviews#5979

Merged

h-vetinari mentioned this pull request Apr 30, 2024

add _numpy_rc conda-forge/staged-recipes#26188

Merged

h-vetinari mentioned this pull request Apr 30, 2024

Add runtime _numpy_rc dependency for rc builds and publish to main conda-forge/numpy-feedstock#314

Merged

This was referenced Apr 30, 2024

NumPy 2.0 support dask/dask#11066

Closed

numexpr v2.10.0 conda-forge/numexpr-feedstock#62

Closed

fix channel_sources ordering; use numpy_rc label

0aeca11

h-vetinari changed the title ~~WIP: numpy 2.0.0rc migration~~ numpy 2.0.0rc migration May 1, 2024

h-vetinari marked this pull request as ready for review May 1, 2024 07:26

h-vetinari requested a review from a team as a code owner May 1, 2024 07:26

h-vetinari mentioned this pull request May 1, 2024

Update pin for numpy #5830

Closed

2 tasks

Provide some more information in commit_message

35c9f8a

This way it also ends up in the PR body, see https://github.com/regro/cf-scripts/blob/a14a4532d2618fc7721aa6ce959639b3c9769dae/conda_forge_tick/migrators/migration_yaml.py#L311

add TL;DR, To-Dos, how-to-use-2.0 & request to keep PR open in case o…

223f244

…f failures

h-vetinari commented May 2, 2024

View reviewed changes

recipe/migrations/pypy38.yaml Outdated Show resolved Hide resolved

jakirkham reviewed May 2, 2024

View reviewed changes

recipe/migrations/numpy2.yaml Show resolved Hide resolved

recipe/migrations/numpy2.yaml Show resolved Hide resolved

recipe/migrations/numpy2.yaml Outdated Show resolved Hide resolved

recipe/migrations/numpy2.yaml Show resolved Hide resolved

h-vetinari and others added 3 commits May 3, 2024 10:26

shorten text a bit

9ffa28b

Merge branch 'main' into numpy2

9c2555c

rewrite To-Dos to make it clearer how to sync with upstream

8dc036b

h-vetinari commented May 3, 2024

View reviewed changes

h-vetinari added 2 commits May 4, 2024 08:17

exclude PyPy from numpy 2.0 migration

cffb06c

turn To-Dos into a heading

d5925f2

h-vetinari merged commit af3dfe7 into conda-forge:main May 4, 2024
4 checks passed

h-vetinari deleted the numpy2 branch May 4, 2024 03:41

h-vetinari commented May 4, 2024

View reviewed changes

xhochy reviewed May 4, 2024

View reviewed changes

h-vetinari mentioned this pull request May 5, 2024

keep formatting of migrator commit_message; consistently use quote indent regro/cf-scripts#2538

Merged

numpy 2.0.0rc migration #5790

numpy 2.0.0rc migration #5790

Conversation

h-vetinari commented Apr 20, 2024 • edited Loading

conda-forge-webservices bot commented Apr 20, 2024

h-vetinari commented Apr 20, 2024

h-vetinari commented Apr 20, 2024

h-vetinari commented Apr 20, 2024

h-vetinari commented Apr 29, 2024

xhochy commented Apr 29, 2024

isuruf commented Apr 29, 2024

h-vetinari commented Apr 30, 2024

xhochy commented Apr 30, 2024

h-vetinari commented Apr 30, 2024

h-vetinari commented May 1, 2024

jakirkham left a comment

Choose a reason for hiding this comment

jakirkham commented May 2, 2024

h-vetinari commented May 2, 2024

Choose a reason for hiding this comment

h-vetinari commented May 3, 2024

h-vetinari commented May 4, 2024

jakirkham commented May 4, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rgommers commented May 4, 2024

h-vetinari commented May 4, 2024

rgommers commented May 4, 2024

h-vetinari commented May 4, 2024

rgommers commented May 4, 2024

h-vetinari commented May 4, 2024

rgommers commented May 4, 2024

h-vetinari commented May 4, 2024 • edited Loading

rgommers commented May 4, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

h-vetinari May 4, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

h-vetinari commented May 5, 2024

h-vetinari commented May 5, 2024

h-vetinari commented Apr 20, 2024 •

edited

Loading

jakirkham commented May 4, 2024 •

edited

Loading

h-vetinari commented May 4, 2024 •

edited

Loading

h-vetinari May 4, 2024 •

edited

Loading