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

Update run script for local builds #12

Merged
merged 2 commits into from
Apr 19, 2021

Conversation

gigony
Copy link
Contributor

@gigony gigony commented Apr 16, 2021

  • Update run script to download GDS from public package (cufile.h is needed to build it locally)
    • Update CMakeLists.txt accordingly
    • GDS is still disabled (need to use GDS Conda package and verify it before enabling GDS)
  • Update CUDA EULA (which include libcufile)
  • Add dependency to yasm which is needed for building libjpeg-turbo
  • Clear CMakeCathe.txt and existing .so files to build with the updated options
  • Minor code change (unnecessarily std::move which degrade performance).
  • Add changelog for v0.18.3 and update the release date of v0.19.0 to 4/19/2021

Verified with a fresh system that https://github.com/rapidsai/cucim/blob/branch-0.20/CONTRIBUTING.md#script-to-build-cucim-from-source is working. (Previously it didn't work due to yasm dependency).

@gigony gigony requested review from a team as code owners April 16, 2021 18:59
@gigony
Copy link
Contributor Author

gigony commented Apr 16, 2021

Hi @raydouglass

Since we didn't tag v0.19.0 yet and VERSION files in branch-0.20 are still referring to 0.19.0, would it make sense to fast-forward branch-0.19 branch to branch-0.20 instead of creating a PR with the same commits to branch-0.19?

If branch-0.19 is not available for fast-forwarding, I can create a PR with the duplicate commits in branch-0.20 for branch-0.19.

@gigony gigony added breaking Introduces a breaking change improvement Improves an existing functionality labels Apr 16, 2021
@gigony
Copy link
Contributor Author

gigony commented Apr 16, 2021

I see some build error because python build is using the existing libcucim conda package but libcucim code in C++ is also changed in the PR.
: https://gpuci.gpuopenanalytics.com/job/rapidsai/job/gpuci-v0.20/job/cucim/job/prb/job/cucim-cpu-python-build/CUDA=11.0,PYTHON=3.7/17/console line 704

Wonder if I need to push PRs for libcucim(core lib code) and cucim (pybind11&python code) separately.

Copy link
Member

@ajschmidt8 ajschmidt8 left a comment

Choose a reason for hiding this comment

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

Approving ops-codeowner file changes

@gigony gigony force-pushed the update_run_script branch 2 times, most recently from c3559c8 to 05d4af6 Compare April 17, 2021 01:09
@gigony
Copy link
Contributor Author

gigony commented Apr 17, 2021

It seems that it always using libcucim from nightly build

https://gpuci.gpuopenanalytics.com/job/rapidsai/job/gpuci-v0.20/job/cucim/job/prb/job/cucim-cpu-python-build/CUDA=11.0,PYTHON=3.7/18/console

18:08:35     libcucim:         0.19.0a210416-cuda11.0_g6cfdbfb_9 rapidsai-nightly  

@ajschmidt8 Do you have any idea on that? How could we handle the situation?

@gigony gigony force-pushed the update_run_script branch 3 times, most recently from d3378c0 to 3948e9b Compare April 18, 2021 23:00
@gigony
Copy link
Contributor Author

gigony commented Apr 19, 2021

I found that libcucim's built package is available at ${WORKSPACE}/ci/artifacts/cucim/cpu/.conda-bld when cucim is built so I updated build.sh for cpu cucim build through cd3ae09, to use cpu libcucim build.
This approach was already made in GPU build (https://github.com/rapidsai/cucim/blob/branch-0.20/ci/gpu/build.sh#L61).

This PR is passing now.

@quasiben, please help review the change.
This change also requires an approval from @jakirkham (rapidsai/cucim-cpp-codeowners) so I don't know how to merge it without @jakirkham's help.

Copy link
Member

@quasiben quasiben left a comment

Choose a reason for hiding this comment

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

Thanks @gigony I'll port these changes back to 0.19 after being merged

@raydouglass raydouglass merged commit 8247296 into rapidsai:branch-0.20 Apr 19, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking Introduces a breaking change improvement Improves an existing functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants