Skip to content

FindcuFile now searches in the current CUDA Toolkit location - #11101

Merged
rapids-bot[bot] merged 2 commits into
NVIDIA:branch-22.08from
robertmaynard:cudfile_search_cuda_location
Jun 16, 2022
Merged

FindcuFile now searches in the current CUDA Toolkit location#11101
rapids-bot[bot] merged 2 commits into
NVIDIA:branch-22.08from
robertmaynard:cudfile_search_cuda_location

Conversation

@robertmaynard

@robertmaynard robertmaynard commented Jun 13, 2022 •
edited
Loading

Copy link
Copy Markdown
Contributor

We shouldn't presume that the CUDA Toolkit is always installed at /usr/local/cuda. Instead we use use find_package(CUDAToolkit) to find the correct location.

@robertmaynard
FindcuFile now searches in the current CUDA Toolkit location
7747dc0 
We shouldn't presume that the CUDA Toolkit is always installed
at /usr/local/cuda. Instead we use use `find_package(CUDAToolkit)` to find the correct location.

In addition since `cufile::cuFile_interface` is part of our public interface we also install the `FindcuFile.cmake` file.
@robertmaynard robertmaynard added bug Something isn't working 3 - Ready for Review Ready for review by team non-breaking Non-breaking change labels Jun 13, 2022
@robertmaynard
robertmaynard requested a review from a team as a code owner June 13, 2022 17:45
@github-actions github-actions Bot added CMake CMake build issue libcudf Affects libcudf (C++/CUDA) code. labels Jun 13, 2022
@vyasr

vyasr commented Jun 14, 2022

Copy link
Copy Markdown
Contributor

In addition since cufile::cuFile_interface is part of our public interface we also install the FindcuFile.cmake file.

Were there meant to be changes aside from adding the search hints in this PR?

@robertmaynard

robertmaynard commented Jun 14, 2022

Copy link
Copy Markdown
Contributor Author

In addition since cufile::cuFile_interface is part of our public interface we also install the FindcuFile.cmake file.

Were there meant to be changes aside from adding the search hints in this PR?

This is a bad commit message. I previously had more export logic but realized it wasn't needed as it was already part of get_cufile.cmake

vyasr
vyasr reviewed Jun 14, 2022
View reviewed changes

@vyasr vyasr left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Needs reformatting, but otherwise LGTM.

@robertmaynard

robertmaynard commented Jun 15, 2022

Copy link
Copy Markdown
Contributor Author

rerun tests

@codecov

codecov Bot commented Jun 15, 2022

Copy link
Copy Markdown

Codecov Report

❗ No coverage uploaded for pull request base (branch-22.08@b7c7adc). Click here to learn what that means.
The diff coverage is n/a.

❗ Current head 76ecd31 differs from pull request most recent head 78b20bc. Consider uploading reports for the commit 78b20bc to get more accurate results

@@               Coverage Diff               @@
##             branch-22.08   #11101   +/-   ##
===============================================
  Coverage                ?   86.34%           
===============================================
  Files                   ?      144           
  Lines                   ?    22738           
  Branches                ?        0           
===============================================
  Hits                    ?    19632           
  Misses                  ?     3106           
  Partials                ?        0

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b7c7adc...78b20bc. Read the comment docs.

@madsbk

madsbk commented Jun 16, 2022

Copy link
Copy Markdown
Contributor

rerun tests

@robertmaynard
robertmaynard requested a review from vyasr June 16, 2022 16:25
@vyasr

vyasr commented Jun 16, 2022

Copy link
Copy Markdown
Contributor

@gpucibot merge

@rapids-bot
rapids-bot Bot merged commit ac2d9a6 into NVIDIA:branch-22.08 Jun 16, 2022
@robertmaynard
robertmaynard deleted the cudfile_search_cuda_location branch June 17, 2022 11:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@vyasr vyasr vyasr approved these changes
+1 more reviewer
@harrism harrism harrism approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

3 - Ready for Review Ready for review by team bug Something isn't working CMake CMake build issue libcudf Affects libcudf (C++/CUDA) code. non-breaking Non-breaking change

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@robertmaynard @vyasr @madsbk @harrism