Skip to content

Change cuda.core license to Apache-2.0 & make contributing guides clear #583

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

Merged
merged 8 commits into from
Apr 28, 2025

Conversation

leofang
Copy link
Member

@leofang leofang commented Apr 25, 2025

Description

Close #533. Close #234.

We implement a short-term solution that retains the NV Software License only to files under cuda_bindings/ and cuda_python/, and change to Apache 2 for cuda_core/ and other files in this repository (ex: helper scripts, CI workflows, ...). Long-term solution regarding the multi-license issue in the repo is TBD.

Checklist

  • New or existing tests cover these changes.
  • The documentation is up to date with these changes.

@leofang leofang added documentation Improvements or additions to documentation support All things related to the project that can't be categorized P0 High priority - Must do! cuda.core Everything related to the cuda.core module labels Apr 25, 2025
@leofang leofang added this to the cuda.core beta 4 milestone Apr 25, 2025
@leofang leofang requested review from aterrel and kkraus14 April 25, 2025 22:27
@leofang leofang self-assigned this Apr 25, 2025
Copy link
Contributor

copy-pr-bot bot commented Apr 25, 2025

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

@leofang
Copy link
Member Author

leofang commented Apr 25, 2025

/ok to test 3978a2f

This comment has been minimized.

@leofang
Copy link
Member Author

leofang commented Apr 26, 2025

/ok to test 58c6f9d

@leofang
Copy link
Member Author

leofang commented Apr 26, 2025

/ok to test 05b63d2

@leofang
Copy link
Member Author

leofang commented Apr 26, 2025

/ok to test 15525a7

@leofang leofang marked this pull request as ready for review April 26, 2025 02:43
@leofang
Copy link
Member Author

leofang commented Apr 26, 2025

@kkraus14 @aterrel this should be ready now

This was referenced Apr 26, 2025
aterrel
aterrel previously approved these changes Apr 26, 2025
@rwgk
Copy link
Collaborator

rwgk commented Apr 26, 2025

LGTM based on what I see under GH "Files changed"

I had the idea to look at the doc preview:

In cuda_bindings/docs/source/index.rst I see this list with 11 items:

   release                                                                      
   install.md                                                                   
   overview.md                                                                  
   motivation.md                                                                
   environment_variables.md                                                     
   api                                                                          
   tips_and_tricks                                                              
   support                                                                      
   contribute.md                                                                
   conduct.md                                                                   
   license                                                                      

Under https://nvidia.github.io/cuda-python/cuda-bindings/latest/ I only see the first 8 items in the side bar, the last 3 are not there. Is that intentional?

It's the same already under https://nvidia.github.io/cuda-python/cuda-bindings/

I.e. without latest/ in the URL.

@leofang
Copy link
Member Author

leofang commented Apr 26, 2025

I am sure we discussed this 😛 the latest docs is updated after a PR is merged into main, not before. AFAIK this is universal behavior with respect to e.g. Read the Docs too. This is why we set up the doc preview CI: #583 (comment)

@rwgk
Copy link
Collaborator

rwgk commented Apr 26, 2025

I am sure we discussed this 😛 the latest docs is updated after a PR is merged into main, not before. AFAIK this is universal behavior with respect to e.g. Read the Docs too. This is why we set up the doc preview CI: #583 (comment)

Oh, I thought I used that link! — I must have fallen off the track when clicking around. Sorry.

@leofang
Copy link
Member Author

leofang commented Apr 26, 2025

No worries at all! There exists a few rough edges in the doc preview I am not happy about, that could lead to what you observed. For example, in some of the places we hard-wire the urls that point to latest. Also because we stitch all the component docs together, the cross links do not respect the url change (having pr-XXX as part of the url). This is why the CI message needs to leave 3 urls instead of 1 🥲 some love is needed for improving it...

@rwgk
Copy link
Collaborator

rwgk commented Apr 26, 2025

Do we need/want contribut.* for cuda-python?

$ git status
On branch license_change
Your branch is up to date with 'leofang/license_change'.
$ find . -type f -name contribut*
./cuda_core/docs/source/contribute.rst
./cuda_bindings/docs/source/contribute.md

@leofang
Copy link
Member Author

leofang commented Apr 26, 2025

I removed it because of #234, but I could use some second opinions here 🙂Let me know what you think...

@rwgk
Copy link
Collaborator

rwgk commented Apr 26, 2025

Minor and unrelated, but JIC you want to fix it while you're at it:

Screenshot 2025-04-26 at 10 23 13

I don't know how those cuda-python v etc. are generated. It looks odd. Maybe just delete the v?

@rwgk
Copy link
Collaborator

rwgk commented Apr 26, 2025

I removed it because of #234, but I could use some second opinions here 🙂Let me know what you think...

Got it, thanks! — I don't have an opinion. Only this much: Might be nice to have for completeness, but not sure if it's worth anyone's time.

@leofang
Copy link
Member Author

leofang commented Apr 26, 2025

I don't know how those cuda-python v etc. are generated. It looks odd. Maybe just delete the v?

Yes, this comes from our own html hack to enable the hidden version selector capability. I opened #587.

@leofang leofang requested a review from kkraus14 April 28, 2025 13:52
Copy link
Collaborator

@kkraus14 kkraus14 left a comment

Choose a reason for hiding this comment

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

We need SPDX identifiers on all of the files in .github as well as the top level config files like pre-commit and bandit configs.

@leofang
Copy link
Member Author

leofang commented Apr 28, 2025

/ok to test ee1dc0a

@leofang leofang requested a review from kkraus14 April 28, 2025 20:37
@leofang
Copy link
Member Author

leofang commented Apr 28, 2025

Merging since both Andy and Keith approved and Ralf also helped review. Thanks guys!

@leofang leofang merged commit bc2e426 into NVIDIA:main Apr 28, 2025
75 checks passed
@leofang leofang deleted the license_change branch April 28, 2025 21:26
Copy link

Doc Preview CI
Preview removed because the pull request was closed or merged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cuda.core Everything related to the cuda.core module documentation Improvements or additions to documentation P0 High priority - Must do! support All things related to the project that can't be categorized
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Change cuda.core license to Apache 2.0 Make CoC and Contributing doc pages per module
4 participants