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

Switch to using cuda-toolkit over cudatoolkit #320

Merged

Conversation

mdemoret-nv
Copy link
Contributor

Description

This PR makes the following changes:

  • Updates dev_env.yml to install cuda-toolkit over cudatoolkit
  • We still have cudatoolkit=11.8 listed at the end. This is necessary to specify which version to use. Some packages still use cudatoolkit and without specifying the version, 11.7 gets installed.
  • Updated the conda build meta.yml to match dev_env.yml
  • Also made some updates to remove warnings and hidden errors
  • Updated the utilities repo to no longer require Cython unless its used
  • Allowed us to remove Cython as a dependency in MRC

Checklist

  • I am familiar with the Contributing Guidelines.
  • New or existing tests cover these changes.
  • The documentation is up to date with these changes.

@mdemoret-nv mdemoret-nv requested review from a team as code owners April 20, 2023 15:53
Copy link
Contributor

@drobison00 drobison00 left a comment

Choose a reason for hiding this comment

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

It looks like Morpheus is having issues when we just install cuda-toolkit, vs cudatookit, on some internal dgx machines, either because CMake isn't reliably finding all the installed headers/libraries, or because they're not being installed correctly.

These changes should probably be tested in a completely clean environment to make sure they work outside of dev/ci.

@dagardner-nv
Copy link
Contributor

It looks like Morpheus is having issues when we just install cuda-toolkit, vs cudatookit, on some internal dgx machines, either because CMake isn't reliably finding all the installed headers/libraries, or because they're not being installed correctly.

I think its the opposite, we're getting build issues when we install cudatookit, which are resolved by installing cuda-toolkit

@mdemoret-nv mdemoret-nv added improvement Improvement to existing functionality breaking Breaking change labels Jun 9, 2023
@codecov
Copy link

codecov bot commented Jun 9, 2023

Codecov Report

Merging #320 (3a10795) into branch-23.07 (7b0d9fa) will decrease coverage by 0.01%.
The diff coverage is n/a.

Additional details and impacted files

Impacted file tree graph

@@               Coverage Diff                @@
##           branch-23.07     #320      +/-   ##
================================================
- Coverage         73.28%   73.27%   -0.01%     
================================================
  Files               390      390              
  Lines             13382    13382              
  Branches           1010     1010              
================================================
- Hits               9807     9806       -1     
- Misses             3575     3576       +1     
Flag Coverage Δ
cpp 69.26% <ø> (-0.01%) ⬇️
py 42.22% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

see 1 file with indirect coverage changes


Continue to review full report in Codecov by Sentry.

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

@drobison00
Copy link
Contributor

It looks like Morpheus is having issues when we just install cuda-toolkit, vs cudatookit, on some internal dgx machines, either because CMake isn't reliably finding all the installed headers/libraries, or because they're not being installed correctly.

I think its the opposite, we're getting build issues when we install cudatookit, which are resolved by installing cuda-toolkit

My bad, I had it backwards :). Thanks David.

Copy link
Contributor

@drobison00 drobison00 left a comment

Choose a reason for hiding this comment

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

LGTM.

@mdemoret-nv
Copy link
Contributor Author

Merging to test with Morpheus. May need updates

@mdemoret-nv
Copy link
Contributor Author

/merge

@rapids-bot rapids-bot bot merged commit a823580 into nv-morpheus:branch-23.07 Jun 9, 2023
@mdemoret-nv mdemoret-nv deleted the mdd_switch-to-cuda-toolkit branch June 9, 2023 22:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking Breaking change improvement Improvement to existing functionality
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

4 participants