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

Add documentation for RAPIDS_NO_INITIALIZE #898

Merged

Conversation

charlesbluca
Copy link
Member

Adds a warning to the UCX docs explaining the hazards of starting a CUDA context before initializing a UCX cluster, and how RAPIDS_NO_INITIALIZE can sometimes be used to avoid this.

@charlesbluca charlesbluca requested a review from a team as a code owner April 21, 2022 19:37
@github-actions github-actions bot added the python python code needed label Apr 21, 2022
@codecov-commenter
Copy link

codecov-commenter commented Apr 21, 2022

Codecov Report

Merging #898 (e10d4b8) into branch-22.06 (7a0c2c2) will not change coverage.
The diff coverage is 100.00%.

❗ Current head e10d4b8 differs from pull request most recent head 97f4fbe. Consider uploading reports for the commit 97f4fbe to get more accurate results

@@              Coverage Diff              @@
##           branch-22.06     #898   +/-   ##
=============================================
  Coverage         89.49%   89.49%           
=============================================
  Files                16       16           
  Lines              2094     2094           
=============================================
  Hits               1874     1874           
  Misses              220      220           
Impacted Files Coverage Δ
dask_cuda/explicit_comms/comms.py 98.87% <ø> (ø)
dask_cuda/local_cuda_cluster.py 88.17% <ø> (ø)
dask_cuda/cuda_worker.py 76.54% <100.00%> (ø)

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 7a0c2c2...97f4fbe. Read the comment docs.

Copy link
Member

@pentschev pentschev left a comment

Choose a reason for hiding this comment

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

Overall looks good, but I requested a few changes to make it more accurate and descriptive. Thanks @charlesbluca .

docs/source/ucx.rst Outdated Show resolved Hide resolved
docs/source/ucx.rst Show resolved Hide resolved
docs/source/ucx.rst Outdated Show resolved Hide resolved
Co-authored-by: Peter Andreas Entschev <[email protected]>
@charlesbluca charlesbluca added 3 - Ready for Review Ready for review by team doc Documentation non-breaking Non-breaking change labels Apr 22, 2022
@charlesbluca
Copy link
Member Author

Thanks @pentschev 🙂 think this should be ready for re-review / approval

Copy link
Member

@pentschev pentschev left a comment

Choose a reason for hiding this comment

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

LGTM, thanks @charlesbluca !

@pentschev
Copy link
Member

@gpucibot merge

@rapids-bot rapids-bot bot merged commit 147043f into rapidsai:branch-22.06 Apr 22, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3 - Ready for Review Ready for review by team doc Documentation non-breaking Non-breaking change python python code needed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants