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.

Sign up for GitHub

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

Jump to bottom

Add a migrator for CUDA 12.8 #7005

Draft

jakirkham wants to merge 1 commit into conda-forge:main from jakirkham-feedstocks:add_cuda128

Member

jakirkham commented Feb 8, 2025

Fixes #6980

This is an initial pass at adding a CUDA 12.8 migrator based on the discussion so far. Based on testing, this works as intended. Though there may be things (like the messaging) that can be improved.

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.


          Add a migrator for CUDA 12.8

Contributor

conda-forge-admin commented Feb 8, 2025

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/meta.yaml) and found it was in an excellent condition.

jakirkham commented

View reviewed changes

Member Author

jakirkham left a comment

Have commented on a few sections below to check we have what we need. Based on testing they work ok, but it is worth checking we have handled the different cases we expect to see throughout conda-forge

recipe/migrations/cuda128.yaml

Comment on lines +26 to +33

+                commit_message: |
+                  Upgrade to CUDA 12.8
+                  With CUDA 12.8, the following new architectures are added `sm_100`, `sm_101` and `sm_120`.
+                  To build for these architectures, maintainers will need to add these to list of architectures
+                  that their package builds for.
+                  ref: https://docs.nvidia.com/cuda/cuda-toolkit-release-notes/index.html#new-features

Member Author

jakirkham Feb 8, 2025

@carterbox could you please take a look at this messaging and suggest any improvements that you have in mind?

recipe/migrations/cuda128.yaml

Comment on lines +41 to +48

+              c_compiler_version:            # [((linux and (x86_64 or aarch64)) or win64) and os.environ.get("CF_CUDA_ENABLED", "False") == "True"]
+                - 13                         # [((linux and (x86_64 or aarch64)) or win64) and os.environ.get("CF_CUDA_ENABLED", "False") == "True"]
+              cxx_compiler_version:          # [((linux and (x86_64 or aarch64)) or win64) and os.environ.get("CF_CUDA_ENABLED", "False") == "True"]
+                - 13                         # [((linux and (x86_64 or aarch64)) or win64) and os.environ.get("CF_CUDA_ENABLED", "False") == "True"]
+              fortran_compiler_version:      # [((linux and (x86_64 or aarch64)) or win64) and os.environ.get("CF_CUDA_ENABLED", "False") == "True"]
+                - 13                         # [((linux and (x86_64 or aarch64)) or win64) and os.environ.get("CF_CUDA_ENABLED", "False") == "True"]

Member Author

jakirkham Feb 8, 2025

Currently this is using GCC 13, but CUDA 12.8 can also support GCC 14. Not sure if we want to update now or wait

Have no personal preferences either way. Just wanted to mention it if we are already planning to upgrade to GCC 14 in the near future

@h-vetinari please let me know if you have thoughts on this

Member

h-vetinari Feb 8, 2025

Just wanted to mention it if we are already planning to upgrade to GCC 14 in the near future

We've settled (roughly) into a rhythm where we wait for GCC {N+1}.0 and the corresponding patch release for GCC N.y before we move to GCC N. For example, GCC 14.1 came out early May 2024, and GCC 13.3 (with the most important fixes that had been accumulated over the course of almost a year) came out end of May 2024. So it's unlikely that we'd move to GCC 14 before June. Also, we still need to fix conda-forge/gfortran_impl_osx-64-feedstock#82

recipe/migrations/cuda128.yaml

Comment on lines +50 to +61

+              docker_image:                                       # [os.environ.get("BUILD_PLATFORM", "").startswith("linux-") and os.environ.get("CF_CUDA_ENABLED", "False") == "True"]
+                # CUDA 12 builds on CentOS 7
+                - quay.io/condaforge/linux-anvil-x86_64:cos7      # [os.environ.get("CF_CUDA_ENABLED", "False") == "True" and os.environ.get("BUILD_PLATFORM") == "linux-64" and os.environ.get("DEFAULT_LINUX_VERSION", "alma9") == "cos7"]
+                - quay.io/condaforge/linux-anvil-aarch64:cos7     # [os.environ.get("CF_CUDA_ENABLED", "False") == "True" and os.environ.get("BUILD_PLATFORM") == "linux-aarch64" and os.environ.get("DEFAULT_LINUX_VERSION", "alma9") == "cos7"]
+                # CUDA 12 builds on AlmaLinux 8
+                - quay.io/condaforge/linux-anvil-x86_64:alma8     # [os.environ.get("CF_CUDA_ENABLED", "False") == "True" and os.environ.get("BUILD_PLATFORM") == "linux-64" and os.environ.get("DEFAULT_LINUX_VERSION", "alma9") in ("alma8", "ubi8")]
+                - quay.io/condaforge/linux-anvil-aarch64:alma8    # [os.environ.get("CF_CUDA_ENABLED", "False") == "True" and os.environ.get("BUILD_PLATFORM") == "linux-aarch64" and os.environ.get("DEFAULT_LINUX_VERSION", "alma9") in ("alma8", "ubi8")]
+                # CUDA 12 builds on AlmaLinux 9
+                - quay.io/condaforge/linux-anvil-x86_64:alma9     # [os.environ.get("CF_CUDA_ENABLED", "False") == "True" and os.environ.get("BUILD_PLATFORM") == "linux-64" and os.environ.get("DEFAULT_LINUX_VERSION", "alma9") == "alma9"]
+                - quay.io/condaforge/linux-anvil-aarch64:alma9    # [os.environ.get("CF_CUDA_ENABLED", "False") == "True" and os.environ.get("BUILD_PLATFORM") == "linux-aarch64" and os.environ.get("DEFAULT_LINUX_VERSION", "alma9") == "alma9"]

Member Author

jakirkham Feb 8, 2025

Have borrowed past migrations's docker_images field and adapted it for our recently updated images ( #6626 )

Would be good to have another pair of eyes on this to make sure we haven't missed anything and to verify we have the intended behavior here

recipe/migrations/cuda128.yaml

Comment on lines +15 to +25

+                ordering:
+                  cuda_compiler:
+                    - None
+                    - nvcc
+                    - cuda-nvcc
+                  cuda_compiler_version:
+                    - None
+                    - 11.8
+                    - 12.4
+                    - 12.6
+                    - 12.8

Member Author

jakirkham Feb 8, 2025

Past migrators have also contained this field. However they included a lot more detail to do the different Docker images we provided for CUDA versions and other details

Since we are at a point where CUDA 12.x reuses a lot of the same content from our global matrix and doesn't change it, found this could be greatly reduced compared to past migrations

jakirkham mentioned this pull request

Adding CUDA 12.8 migration #6980

Open

jakirkham commented

View reviewed changes

recipe/migrations/cuda128.yaml

Comment on lines +50 to +61

+              docker_image:                                       # [os.environ.get("BUILD_PLATFORM", "").startswith("linux-") and os.environ.get("CF_CUDA_ENABLED", "False") == "True"]
+                # CUDA 12 builds on CentOS 7
+                - quay.io/condaforge/linux-anvil-x86_64:cos7      # [os.environ.get("CF_CUDA_ENABLED", "False") == "True" and os.environ.get("BUILD_PLATFORM") == "linux-64" and os.environ.get("DEFAULT_LINUX_VERSION", "alma9") == "cos7"]
+                - quay.io/condaforge/linux-anvil-aarch64:cos7     # [os.environ.get("CF_CUDA_ENABLED", "False") == "True" and os.environ.get("BUILD_PLATFORM") == "linux-aarch64" and os.environ.get("DEFAULT_LINUX_VERSION", "alma9") == "cos7"]
+                # CUDA 12 builds on AlmaLinux 8
+                - quay.io/condaforge/linux-anvil-x86_64:alma8     # [os.environ.get("CF_CUDA_ENABLED", "False") == "True" and os.environ.get("BUILD_PLATFORM") == "linux-64" and os.environ.get("DEFAULT_LINUX_VERSION", "alma9") in ("alma8", "ubi8")]
+                - quay.io/condaforge/linux-anvil-aarch64:alma8    # [os.environ.get("CF_CUDA_ENABLED", "False") == "True" and os.environ.get("BUILD_PLATFORM") == "linux-aarch64" and os.environ.get("DEFAULT_LINUX_VERSION", "alma9") in ("alma8", "ubi8")]
+                # CUDA 12 builds on AlmaLinux 9
+                - quay.io/condaforge/linux-anvil-x86_64:alma9     # [os.environ.get("CF_CUDA_ENABLED", "False") == "True" and os.environ.get("BUILD_PLATFORM") == "linux-64" and os.environ.get("DEFAULT_LINUX_VERSION", "alma9") == "alma9"]
+                - quay.io/condaforge/linux-anvil-aarch64:alma9    # [os.environ.get("CF_CUDA_ENABLED", "False") == "True" and os.environ.get("BUILD_PLATFORM") == "linux-aarch64" and os.environ.get("DEFAULT_LINUX_VERSION", "alma9") == "alma9"]

Member Author

jakirkham Feb 8, 2025

Looks like pre-commit doesn't like the line length of this section based on this CI error

recipe/migrations/cuda128.yaml
  52:181    error    line too long (222 > 180 characters)  (line-length)
  53:181    error    line too long (227 > 180 characters)  (line-length)
  56:181    error    line too long (233 > 180 characters)  (line-length)
  57:181    error    line too long (238 > 180 characters)  (line-length)
  60:181    error    line too long (223 > 180 characters)  (line-length)
  61:181    error    line too long (228 > 180 characters)  (line-length)

Not sure we can fix that give the selectors needed. Maybe we can disable this check?

Member

h-vetinari Feb 8, 2025

Yeah, that check needs to be ignored or disabled. The docker selection is going to stay this complicated until we drop CUDA 11.8.

h-vetinari reviewed

View reviewed changes

Member

h-vetinari left a comment

This is an initial pass at adding a CUDA 12.8 migrator based on the discussion so far.

I don't feel like my concern has been addressed, so I reiterated it in the issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet