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

Various fixes for build.sh #771

Merged
merged 5 commits into from
Aug 2, 2022

Conversation

vyasr
Copy link
Contributor

@vyasr vyasr commented Aug 2, 2022

This PR includes the following fixes for build.sh:

  • The parallel argument is for the build tool (make/ninja) and not CMake or setup.py, so it needs to be passed after a second --.
  • pylibraft requires the distances component of libraft to be compiled. If pylibraft is passed to build.sh, we need to force building libraft with the distance component
  • build.sh doesn't install by default, so the CMAKE_PREFIX_PATH that the Python build points to needs to also include the build directory in addition to the install directory. I think giving the build directory precedence makes more sense, but that could be changed if necessary.

@vyasr vyasr added bug Something isn't working 3 - Ready for Review non-breaking Non-breaking change labels Aug 2, 2022
@vyasr vyasr self-assigned this Aug 2, 2022
@vyasr vyasr requested a review from a team as a code owner August 2, 2022 17:07
@vyasr vyasr added 5 - DO NOT MERGE Hold off on merging; see PR for details and removed 3 - Ready for Review labels Aug 2, 2022
@vyasr vyasr changed the title Fix parallelism for build.sh for python Various fixes for build.sh Aug 2, 2022
@vyasr vyasr added 3 - Ready for Review and removed 5 - DO NOT MERGE Hold off on merging; see PR for details labels Aug 2, 2022
@teju85
Copy link
Member

teju85 commented Aug 2, 2022

I confirm that this PR fixes the issues I was facing: (Thanks @vyasr !)

  1. parallel build not working
  2. "distances" sources getting compiled twice (due to me having to set FIND_RAFT_CPP=OFF)

After this PR gets merged, if one wants reuse the compiled sources from cpp/src, then one needs to make sure NOT to pass the above FIND_RAFT_CPP=OFF option!

That said, there's still a minor hiccup: the python setup.py ... commands in build.sh somehow doesn't seem to honor setting up ccache through options like -DCMAKE_CUDA_COMPILER_LAUNCHER=ccache. For now, this is not an issue for me as I'll not be choosing the FIND_RAFT_CPP=OFF path.

@teju85
Copy link
Member

teju85 commented Aug 2, 2022

@gpucibot merge

@rapids-bot rapids-bot bot merged commit b0fdebb into rapidsai:branch-22.10 Aug 2, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3 - Ready for Review bug Something isn't working non-breaking Non-breaking change
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants