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

Add classifiers for CUDA 11 / 12 / 12.0 #124

Merged
merged 2 commits into from
Dec 22, 2022
Merged

Add classifiers for CUDA 11 / 12 / 12.0 #124

merged 2 commits into from
Dec 22, 2022

Conversation

leofang
Copy link
Contributor

@leofang leofang commented Dec 9, 2022

Request to add a new Trove classifier.

The name of the classifier(s) you would like to add:

Why do you want to add this classifier?

CUDA 12.0 just came out today, so it'd be nice to add it now.

As for the major-version classifiers (CUDA 11/12), I consider them useful because starting with CUDA 11 there is guaranteed minor version compatibility at the programming model level (with caveats that users/developers would understand), so wheels can tag CUDA :: 11 or CUDA :: 12 to declare such compatibility. For example, NVIDIA and some GPU frameworks have been uploading wheels with suffix -cuXX (XX=11 currently) on PyPI.org.

cc: @jakirkham @m3vaz

@jakirkham
Copy link

LGTM. Thanks Leo! 🙏

@jakirkham
Copy link

cc @di

@di
Copy link
Member

di commented Dec 9, 2022

I feel like having both 12 and 12.0 will be confusing. I realize this is what we do for the Python classifiers, but there's a lot more precedent there.

Would you be opposed to:

  • Environment :: GPU :: NVIDIA CUDA :: 11
  • Environment :: GPU :: NVIDIA CUDA :: 12
  • Environment :: GPU :: NVIDIA CUDA :: 12 :: 12.0

instead? Or is there another way to refer to everything compatible with a major version?

@leofang
Copy link
Contributor Author

leofang commented Dec 9, 2022

Thanks, Dustin. I don't have strong opinion about Environment :: GPU :: NVIDIA CUDA :: 12.0 vs Environment :: GPU :: NVIDIA CUDA :: 12 :: 12.0, except that I'd note the former is following the years of established precedence. Having the extra major ver in the middle (as in CUDA :: 12 :: 12.0) does not seem to convey extra information.

Also, we're not changing any existing Environment :: GPU :: NVIDIA CUDA :: 11.x to Environment :: GPU :: NVIDIA CUDA :: 11 :: 11.x, right? So from this point of view, I'd argue the latter CUDA :: 12 :: 12.0 is more confusing than the former.

But in any case I agree that we are creating a new convention (for major-ver only classifiers). As I said I don't have strong opinion 🙂 Perhaps @jakirkham or @m3vaz could chime in?

@di
Copy link
Member

di commented Dec 9, 2022

Would this be more or less confusing in your opinion?

  • Environment :: GPU :: NVIDIA CUDA :: 11.x
  • Environment :: GPU :: NVIDIA CUDA :: 12.x
  • Environment :: GPU :: NVIDIA CUDA :: 12.0

I don't feel too strongly about this, but whatever we decide will be set in stone, more or less.

@leofang
Copy link
Contributor Author

leofang commented Dec 9, 2022

That's interesting, I am thinking 11.x/12.x are more colloquial terms and a bit uncertain if its semantics is obvious from the first glance.

but whatever we decide will be set in stone, more or less.

Indeed. I wouldn't mind if you want to keep this PR open for a bit longer, Dustin. I can also ask NV folks internally and see if any of them has strong opinion on the 3 proposals (this, this, this) on the table.

Thanks!

@leofang
Copy link
Contributor Author

leofang commented Dec 12, 2022

Hi @di we discussed internally briefly about this, with CUDA product management involved. The 2nd proposal is slightly preferred (though none of us has strong opinion), so I just updated the PR. PTAL 🙂 thx!

@leofang
Copy link
Contributor Author

leofang commented Dec 14, 2022

Friendly nudge @di 🙂

@leofang
Copy link
Contributor Author

leofang commented Dec 22, 2022

Another friendly nudge @di 🙂

@di di merged commit 9a90c33 into pypa:main Dec 22, 2022
@leofang leofang deleted the new_cuda branch December 22, 2022 04:05
@jakirkham
Copy link

Thanks Leo and Dustin! 🙏

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.

3 participants