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.

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

Allow multiple 'local' installs of cuda to be specified #66

Merged
merged 9 commits into from
Apr 12, 2023
Merged

Allow multiple 'local' installs of cuda to be specified #66

merged 9 commits into from
Apr 12, 2023

Conversation

jsharpe
Copy link
Member

@jsharpe jsharpe commented Feb 5, 2023

Exposes the local_cuda repository_rule to allow additional installs of cuda to be installed as toolchains.
e.g.

local_cuda(name="cuda_11_4", toolkit_path = "/usr/local/cuda-11.4")

@jsharpe jsharpe mentioned this pull request Feb 5, 2023
Closed
@jsharpe jsharpe marked this pull request as draft February 5, 2023 00:14
fmeum
fmeum approved these changes Feb 5, 2023
View reviewed changes
cuda/extensions.bzl Outdated Show resolved Hide resolved
@jsharpe jsharpe mentioned this pull request Feb 5, 2023
Merged
@cloudhan
Copy link
Collaborator

cloudhan commented Feb 8, 2023 •
edited
Loading

Is it still draft? I think this should good to go?

@jsharpe jsharpe marked this pull request as ready for review March 20, 2023 23:23
@jsharpe jsharpe mentioned this pull request Mar 20, 2023
Draft
@cloudhan
Copy link
Collaborator

It is weird that the CI failure cannot be reproduced on local machine with cuda 11.*

cloudhan
cloudhan reviewed Mar 22, 2023
View reviewed changes
cuda/extensions.bzl Outdated
@@ -2,7 +2,28 @@

load("//cuda/private:repositories.bzl", "local_cuda")

cuda_toolkit = tag_class(attrs = {
"name": attr.string(doc = "Name for the toolchain repository", default = "local_cuda"),
"toolkit_path": attr.string(doc = "Path to the CUDA SDK"),
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit, attr is default to mandatory=False, in this case, local_cuda will do the detect. We shall add "..., otherwise, automatically detect it blahblah..."

@cloudhan
Copy link
Collaborator

cloudhan commented Mar 29, 2023 •
edited
Loading

@jsharpe Finally got sometime to figure this one out. The reason is a mixture of CI and you impl. On CI we only installed cuda-nvcc-10-1 cuda-cudart-dev-10-1, then /usr/local/cuda is not configured so only /usr/local/cuda-10.1 exists. And in you change cuda.local_toolchain(name = "local_cuda", toolkit_path = "/usr/local/cuda") cause bazel creating a dangling reference. Can you change it default to toolkit_path = ""? So that the env var CUDA_PATH will be picked up.

cloudhan
cloudhan reviewed Mar 29, 2023
View reviewed changes
examples/MODULE.bazel Outdated Show resolved Hide resolved
@jsharpe jsharpe requested a review from cloudhan April 11, 2023 22:51
cloudhan
cloudhan approved these changes Apr 12, 2023
View reviewed changes
Copy link
Collaborator

@cloudhan cloudhan left a comment

Choose a reason for hiding this comment

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

Thanks.

@jsharpe jsharpe merged commit c0f34ce into bazel-contrib:main Apr 12, 2023
@jsharpe jsharpe deleted the local_cuda branch July 14, 2023 09:36
@cloudhan cloudhan mentioned this pull request Jul 17, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@fmeum fmeum fmeum approved these changes

@cloudhan cloudhan cloudhan approved these changes

@ryanleary ryanleary Awaiting requested review from ryanleary ryanleary is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@jsharpe @cloudhan @fmeum