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

Fix nvcc command lookup #3028

Merged
merged 1 commit into from Feb 6, 2020
Merged

Fix nvcc command lookup #3028

merged 1 commit into from Feb 6, 2020

Conversation

niboshi
Copy link
Member

@niboshi niboshi commented Feb 5, 2020

Takes over #3023

In some environment, nvcc is not necessarilly looked up byPATH.

@niboshi niboshi mentioned this pull request Feb 5, 2020
Closed
@niboshi
Copy link
Member Author

niboshi commented Feb 5, 2020

I used the logic of the functions with the same name in install/build.py but rewrote entirely.

@niboshi niboshi added the cat:bug Bugs label Feb 5, 2020
@asi1024 asi1024 added this to the v8.0.0a1 milestone Feb 5, 2020
@asi1024 asi1024 added the to-be-backported Pull-requests to be backported to stable branch label Feb 5, 2020
asi1024
asi1024 reviewed Feb 5, 2020
View reviewed changes
cupy/cuda/_environment.py
_nvcc_path = ()


def get_cuda_path():
Copy link
Member

Choose a reason for hiding this comment

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

How about naming like as get_cuda_path_with_cache?

Copy link
Member Author

Choose a reason for hiding this comment

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

I think caching is an internal behavior and it's not important for the caller.

@asi1024
Copy link
Member

asi1024 commented Feb 6, 2020

Jenkins, test this please.

@pfn-ci-bot
Copy link
Collaborator

Successfully created a job for commit c808641:

@chainer-ci
Copy link
Member

Jenkins CI test (for commit c808641, target branch master) succeeded!

@asi1024
Copy link
Member

asi1024 commented Feb 6, 2020

LGTM.

@asi1024 asi1024 merged commit 9a3ab93 into cupy:master Feb 6, 2020
@asi1024 asi1024 mentioned this pull request Feb 6, 2020
Merged
asi1024 added a commit to asi1024/cupy that referenced this pull request Feb 6, 2020
@asi1024
Merge pull request cupy#3028 from niboshi/lookup-nvcc
7611887 
Fix nvcc command lookup
@niboshi niboshi deleted the lookup-nvcc branch February 6, 2020 09:10
@leofang
Copy link
Member

leofang commented Feb 10, 2020

Hi @niboshi, could you briefly comment what's the difference between cupy/cuda/_environment.py and cupy/_environment.py? Both files have get_cuda_path().

@niboshi
Copy link
Member Author

niboshi commented Feb 12, 2020

Oh, I didn't notice there's one in cupy/_environment.py.
I think they better fit cupy/cuda/_environment.py because they're related to CUDA environment (especially considering the possibility to make them public in the future).

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

@asi1024 asi1024 asi1024 left review comments

Assignees

@asi1024 asi1024

Labels
cat:bug Bugs to-be-backported Pull-requests to be backported to stable branch
Projects
None yet
Milestone
v8.0.0a1
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@niboshi @asi1024 @pfn-ci-bot @chainer-ci @leofang