-
Notifications
You must be signed in to change notification settings - Fork 22
Add CUPTI Python API #479
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 CUPTI Python API #479
Conversation
|
Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually. Contributors can view more details about this message here. |
|
/ok to test |
madsbk
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall looks good
Co-authored-by: Mads R. B. Kristensen <[email protected]>
Co-authored-by: Mads R. B. Kristensen <[email protected]>
This reverts commit deb328e.
madsbk
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good @pentschev, I only have some minor stuff
pentschev
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Improve formatting
This reverts commit 558c82c.
Co-authored-by: Mads R. B. Kristensen <[email protected]>
| - ${{ compiler("c") }} | ||
| - ${{ compiler("cxx") }} | ||
| - ${{ compiler("cuda") }} | ||
| - cuda-cupti-dev |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We typically have only compilers in build and all CUDA libraries (e.g. libcublas-dev, cuda-cupti-dev, etc.) in host. (Because the compiler depends on cuda-cudart-dev, it ends up in both build and host.) One way to think about it is that if you were cross-compiling, you'd need your toolchain like the compiler executables in build but all the libraries you're building against go in host. For this reason I suspect we should only need cuda-cupti-dev in host, and that if it isn't working that way, we have some kind of build system bug that needs to be identified and addressed.
For now, I don't want to block further work, but let's flag this:
| - cuda-cupti-dev | |
| - cuda-cupti-dev # TODO: This should only be needed in host, and may indicate a packaging issue |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Following a discussion offline, instead of adding comments we started #510 to track this, given this PR is only an extension of changes already added in #445 , where cuda-cupti-dev is part of build section of librapidsmpf's conda recipe.
|
Thanks all for the reviews! |
|
/merge |
No description provided.