-
Notifications
You must be signed in to change notification settings - Fork 58
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
cufile version #565
cufile version #565
Conversation
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.
I think I understand the goals here, but can you make the PR description clearer? Something like “we observed a failure on CTK 12.0 because …”. Or file an issue for this PR.
Added #566 |
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.
Tiny typo suggestion
Co-authored-by: Lawrence Mitchell <wence@gmx.li>
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.
Some minor suggestions but otherwise LGTM.
@@ -10,6 +11,24 @@ | |||
DriverProperties = cufile_driver.DriverProperties | |||
|
|||
|
|||
def libcufile_version() -> Tuple[int, int]: | |||
"""Get the libcufile version (or zero if older than v1.8). |
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.
Minor note that this returns (0, 0) and not 0 for old versions. I would suggest removing the parenthetical here and instead specifying that behavior in the Notes below.
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.
Changed to:
def libcufile_version() -> Tuple[int, int]:
"""Get the libcufile version.
Returns (0, 0) for cuFile versions prior to v1.8.
Notes
-----
This is not the version of the CUDA toolkit. cufile is part of the
toolkit but follows its own version scheme.
Returns
-------
The version as a tuple (MAJOR, MINOR).
"""
Co-authored-by: Vyas Ramasubramani <vyas.ramasubramani@gmail.com> Co-authored-by: Bradley Dice <bdice@bradleydice.com>
Thanks everyone |
Co-authored-by: jakirkham <jakirkham@gmail.com>
/merge |
Fixes #566
To clean up and make cufile capability detection more robust, we now use
cuFileGetVersion()
.Should fix the nightly failure: https://github.com/rapidsai/kvikio/actions/runs/12133477083/job/33877440755
NB:
cuFileGetVersion()
first became available in cufile v1.8 (CTK v12.3) thus the stream and batch API detection will return false for versions older than v1.8. I think this is acceptable for robustness.