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

Close CUDA 12.0 migration #5613

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

jakirkham
Copy link
Member

@jakirkham jakirkham commented Mar 9, 2024

Now that a large majority of feedstocks have been rebuilt or have an open migration PR, let's go ahead and close out this PR and add CUDA 12 to the matrix.

This adds CUDA 12.0 & CUDA 12.4. Where CUDA 12.0 represents the oldest CUDA 12 version built against and supported in conda-forge. Similarly CUDA 12.4 represents the latest CUDA version in conda-forge. This way if users are still have packages that depend on an older CUDA 12 version, packages built with CUDA 12.0 will satisfy that need. Also if users are trying to take advantage of packages built with the latest optimizations, they can install packages built with the latest CUDA Toolkit in conda-forge. We keep the latest version pinned to avoid pulling in partially rolled out CUDA Toolkit updates. Meaning we will need to go back through and bump this number when a new CUDA Toolkit becomes available.

xref: #5390


Checklist

  • Used a personal fork of the feedstock to propose changes
  • Bumped the build number (if the version is unchanged)
  • Reset the build number to 0 (if the version changed)
  • Re-rendered with the latest conda-smithy (Use the phrase @conda-forge-admin, please rerender in a comment in this PR for automated rerendering)
  • Ensured the license file is being packaged.

Now that a large majority of feedstocks have been rebuilt or have an
open migration PR, let's go ahead and close out this PR and add CUDA 12
to the matrix.

This adds CUDA 12.0 & CUDA 12.4. Where CUDA 12.0 represents the oldest
CUDA 12 version built against and supported in conda-forge. Similarly
CUDA 12.4 represents the latest CUDA version in conda-forge. This way if
users are still have packages that depend on an older CUDA 12 version,
packages built with CUDA 12.0 will satisfy that need. Also if users are
trying to take advantage of packages built with the latest
optimizations, they can install packages built with the latest CUDA
Toolkit in conda-forge. We keep the latest version pinned to avoid
pulling in partially rolled out CUDA Toolkit updates. Meaning we will
need to go back through and bump this number when a new CUDA Toolkit
becomes available.
@conda-forge-webservices
Copy link
Contributor

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.

@jakirkham
Copy link
Member Author

cc @conda-forge/core

cuda_compiler_version:
- None
- 11.2 # [(linux or win64) and os.environ.get("CF_CUDA_ENABLED", "False") == "True"]
- 11.8 # [(linux or win64) and os.environ.get("CF_CUDA_ENABLED", "False") == "True"]
- 12.0 # [(linux or win64) and os.environ.get("CF_CUDA_ENABLED", "False") == "True"]
- 12.4 # [(linux or win64) and os.environ.get("CF_CUDA_ENABLED", "False") == "True"]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are packages built for 12.0 expected to always successfully build with 12.4? Shouldn't we separate the closing of the 12.0 migration from adding 12.4...?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I remember that in some feedstocks I had to add some code to explicitly use 12.0 to compile. This code should work with 12.4 but as it is not tested, I would feel better if we had a migrator first.

Copy link
Member

@h-vetinari h-vetinari left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we should add 12.4 here; I'd be open to dropping 11.2 in the same PR though.

Comment on lines +178 to +183
# case: native compilation (build == target)
- quay.io/condaforge/linux-anvil-ppc64le # [ppc64le and os.environ.get("BUILD_PLATFORM") == "linux-ppc64le"]
- quay.io/condaforge/linux-anvil-aarch64 # [aarch64 and os.environ.get("BUILD_PLATFORM") == "linux-aarch64"]
# case: cross-compilation (build != target)
- quay.io/condaforge/linux-anvil-cos7-x86_64 # [ppc64le and os.environ.get("BUILD_PLATFORM") == "linux-64"]
- quay.io/condaforge/linux-anvil-cos7-x86_64 # [aarch64 and os.environ.get("BUILD_PLATFORM") == "linux-64"]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm also pretty sure that the selectors here need os.environ.get("CF_CUDA_ENABLED", "False") == "True" (which was active in the migrator through the selector on docker_images).

@h-vetinari
Copy link
Member

I think we should move forward here with dropping 11.2 and adding 12.0. Let's split off 12.4.

@h-vetinari h-vetinari mentioned this pull request Apr 23, 2024
Copy link
Member

@h-vetinari h-vetinari left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The announced date for dropping 11.2 has now passed; given that and the various unanswered review requests here, I've opened #5799 which builds on this PR and incorporates them.

Comment on lines +11 to +12
- 12 # [os.environ.get("cf_cuda_enabled", "false") == "true" and linux]
- 12 # [os.environ.get("cf_cuda_enabled", "false") == "true" and linux]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is incorrectly lower-cased here (that environment variable is not set by smithy)

cuda_compiler_version:
- None
- 11.2 # [(linux or win64) and os.environ.get("CF_CUDA_ENABLED", "False") == "True"]
- 11.8 # [(linux or win64) and os.environ.get("CF_CUDA_ENABLED", "False") == "True"]
- 12.0 # [(linux or win64) and os.environ.get("CF_CUDA_ENABLED", "False") == "True"]
- 12.4 # [(linux or win64) and os.environ.get("CF_CUDA_ENABLED", "False") == "True"]
cuda_compiler_version_min:
- None # [osx]
- 11.2 # [linux or win64]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This also needs to be bumped (assuming we drop CUDA 11.2 here).

@h-vetinari
Copy link
Member

h-vetinari commented May 1, 2024

If you plan to rebase this PR now that #5799 is in, you can effectively condense it to a revert of 688ab0d (at least for 12.0; you'd have to add the 12.4 changes on top, though I still don't think they belong into this PR)

@h-vetinari
Copy link
Member

@jakirkham, what's the status here? What still needs to be done from your POV? (since you asked not to close this migration as part of #5799)

@h-vetinari
Copy link
Member

@jakirkham, what's the status here? What still needs to be done from your POV?

Gentle ping on this

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