-
Notifications
You must be signed in to change notification settings - Fork 158
Lazy load code modules #269
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
Changes from all commits
3e15ebc
6ac39ba
016055e
9fba2b7
a55f322
d0d528b
a2555f9
e500304
f9b3b91
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -7,20 +7,12 @@ | |
# is strictly prohibited. | ||
|
||
import pytest | ||
from conftest import can_load_generated_ptx | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. My understanding is that we don't need explicit imports for things from conftest? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I believe this only aplies to fixtures. helper functions still need to be imported. This is what my research and tests have shown. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I could add a new file called utils, or helpers and import that instead. There is mixed opinions on best practice online There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
I was only relying on chatgpt before. It told me the imports are not needed. If that's not true: keeping it simple seems best to me, unless you anticipate that we're accumulating many helper functions. I.e. just keep what you have? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yeah, it told me that too, then changed its mind. Keeping it simple was my idea as well, with only 1 helper function, it seems premature to split it into a new file, but perhaps down the line it will make sense There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yeah we can move it to |
||
|
||
from cuda import cuda, nvrtc | ||
from cuda.core.experimental import Device, Program | ||
from cuda.core.experimental._module import Kernel, ObjectCode | ||
|
||
|
||
def can_load_generated_ptx(): | ||
_, driver_ver = cuda.cuDriverGetVersion() | ||
_, nvrtc_major, nvrtc_minor = nvrtc.nvrtcVersion() | ||
if nvrtc_major * 1000 + nvrtc_minor * 10 > driver_ver: | ||
return False | ||
return True | ||
|
||
|
||
def test_program_init_valid_code_type(): | ||
code = 'extern "C" __global__ void my_kernel() {}' | ||
program = Program(code, "c++") | ||
|
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.
good catch thanks
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.
don't thank me, thank the good folks at ruff