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

Reduce cuda complexity by dropping 11.8 #6917

Closed
wants to merge 1 commit into from

Conversation

hmaarrfk
Copy link
Contributor

For years we have struggled in maintainning the multiple cuda versions.

Personally, I feel that Nvidia has done tremendous work in ameliorating the situation here at Conda-forge.

However, I'm hoping to shed a little of bit of mental weight from cuda 11.8.

IMO the main projects that utilize it:

have all dropped CUDA 11.8 for lack of maintainer interest.

I think it is time for us to drop it from the global pinnings

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.

For years we have struggled in maintainning the multiple cuda versions.

Personally, I feel that Nvidia has done tremendous work in ameliorating
the situation here at Conda-forge.

However, I'm hoping to shed a little of bit of mental weight from cuda
11.8.

IMO the main projects that utilize it:
* pytorch - https://github.com/conda-forge/pytorch-cpu-feedstock/blob/main/recipe/meta.yaml#L58
* tensorflow - https://github.com/conda-forge/tensorflow-feedstock/blob/main/recipe/meta.yaml#L60
* jax - https://github.com/conda-forge/jaxlib-feedstock/blob/main/recipe/meta.yaml#L27
* onnxruntime https://github.com/conda-forge/onnxruntime-feedstock/blob/main/recipe/meta.yaml#L48

have all dropped CUDA 11.8 for lack of maintainer interest.

I think it is time for us to drop it from the global pinnings
cuda_compiler:
- None
- nvcc # [(linux or win64) and os.environ.get("CF_CUDA_ENABLED", "False") == "True"]
- cuda-nvcc # [(linux or win64) and os.environ.get("CF_CUDA_ENABLED", "False") == "True"]
Copy link
Contributor Author

Choose a reason for hiding this comment

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

do we really need to use this key? could we have users simply depend on cuda-nvcc? I likely don't understand the downstream implications.

Copy link
Member

Choose a reason for hiding this comment

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

Users shouldn't depend on cuda-nvcc but continue to depend on {{ compiler("cuda") }}. For that, we need to set cuda_compiler in the pinning, also in a 12+ world

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

Copy link
Contributor

@ngam ngam left a comment

Choose a reason for hiding this comment

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

Shedding nice-to-haves is always good 👍

@hmaarrfk hmaarrfk changed the title Reduce cuda complexity dry dropping 11.8 Reduce cuda complexity by dropping 11.8 Jan 13, 2025
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.

Neither for nor against the 11.8 removal, but this PR is not yet correct for its stated goal.

cuda_compiler:
- None
- nvcc # [(linux or win64) and os.environ.get("CF_CUDA_ENABLED", "False") == "True"]
- cuda-nvcc # [(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.

Users shouldn't depend on cuda-nvcc but continue to depend on {{ compiler("cuda") }}. For that, we need to set cuda_compiler in the pinning, also in a 12+ world

Comment on lines -180 to -185
- cuda_compiler # [linux and os.environ.get("CF_CUDA_ENABLED", "False") == "True"]
- cuda_compiler_version # [linux and os.environ.get("CF_CUDA_ENABLED", "False") == "True"]
- docker_image # [linux and os.environ.get("CF_CUDA_ENABLED", "False") == "True" and os.environ.get("BUILD_PLATFORM", "").startswith("linux-")]
- # [win64 and os.environ.get("CF_CUDA_ENABLED", "False") == "True"]
- cuda_compiler # [win64 and os.environ.get("CF_CUDA_ENABLED", "False") == "True"]
- cuda_compiler_version # [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.

This will thus continue to be necessary

Comment on lines 51 to +53
cuda_compiler_version:
- None
- 11.8 # [(linux or win64) and os.environ.get("CF_CUDA_ENABLED", "False") == "True"]
- 12.4 # [linux and ppc64le and os.environ.get("CF_CUDA_ENABLED", "False") == "True"]
- 12.6 # [((linux and not ppc64le) or win64) and os.environ.get("CF_CUDA_ENABLED", "False") == "True"]
- 12 # [(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.

I don't want to switch to free-floating minor for 12.x in an unrelated PR. If you think that's a good idea, please open an issue and we can discuss it there.

Copy link
Member

@jakirkham jakirkham Jan 14, 2025

Choose a reason for hiding this comment

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

Right we also discussed this previously. The reason we pin this here is to avoid picking up incomplete CUDA Toolkit's during upgrades

We do plan to bump this version whenever the next CUDA Toolkit is fully available on conda-forge

@jakirkham
Copy link
Member

jakirkham commented Jan 14, 2025

Thanks for the kind words Mark! 🙏

Am glad to hear the CUDA 12 packages are serving the community well 😊

Can understand individual feedstock maintainers taking the step of shedding maintenance burden. And they certainly should feel free to skip versions they no longer can or want to support

However when it comes to the global matrix in conda-forge, we have been taking the approach of keeping a broad range of versions. For example with Python we have been keeping all versions that have not reached EOL

By the same measure we have kept around CUDA 11 (specifically 11.8). We certainly have tried to cutdown on the burden by dropping old CUDAs ( #1708 ), moving to CUDA 11.8 ( #4834 ), and dropping anything previous ( #5799 ). Not to mention the significant collective effort put into migrating feedstocks. All this to ensure only one version of CUDA 11 (the latest one) is around

We do this in part because CUDA 11.8 is the last version that still supports Kepler. In CUDA 12.0, Kepler support was removed. Once we drop CUDA 11.8, those users will no longer have support

NVIDIA continues to publish its own libraries (like CUDA Python and RAPIDS with CUDA 11 support). Also a number of packages in conda-forge either from NVIDIA or maintained in part by NVIDIA, ship with CUDA 11 support, which NVIDIA helps maintain

Looking through conda-forge, there are a number of feedstocks that continue to support CUDA 11. So it seems probable that others still value providing support for CUDA 11 too

While I understand why individual maintainers would like to drop CUDA 11 from their feedstocks (and they certainly should feel free to using skips). In terms of many folks still maintaining CUDA 11, it is much easier to do so with it in the global matrix

Would appreciate hearing a bit more how this issue is coming up for you. Perhaps we can find another acceptable solution

@hmaarrfk hmaarrfk closed this Jan 14, 2025
@hmaarrfk
Copy link
Contributor Author

Thanks for the thorough explanation

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.

5 participants