-
Notifications
You must be signed in to change notification settings - Fork 279
Make Linker.backend a classmethod #1910
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
base: main
Are you sure you want to change the base?
Changes from all commits
432771c
6327b22
12d291b
af29cd9
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 |
|---|---|---|
|
|
@@ -167,10 +167,22 @@ cdef class Linker: | |
| else: | ||
| return as_py(self._culink_handle) | ||
|
|
||
| @property | ||
| def backend(self) -> str: | ||
| """Return this Linker instance's underlying backend.""" | ||
| return "nvJitLink" if self._use_nvjitlink else "driver" | ||
| @classmethod | ||
| def backend(cls) -> str: | ||
| """Return which linking backend will be used. | ||
|
|
||
| Returns ``"nvJitLink"`` when the nvJitLink library is available | ||
| and meets the minimum version requirement, otherwise ``"driver"``. | ||
|
|
||
| .. note:: | ||
|
|
||
| Prefer letting :class:`Linker` decide. Query ``backend()`` | ||
| only when you need to dispatch based on input format (for | ||
| example: choose PTX vs. LTOIR before constructing a | ||
| ``Linker``). The returned string names an implementation | ||
| detail whose support matrix may shift across CTK releases. | ||
| """ | ||
| return "driver" if _decide_nvjitlink_or_driver() else "nvJitLink" | ||
|
Comment on lines
-170
to
+185
Contributor
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. We did just complete a sweep to standardize the property/method distinction. It would be great to keep that as clean as possible. One option is to roll our own classproperty: class classproperty:
def __init__(self, func):
self.func = func
def __get__(self, instance, owner):
return self.func(owner)
class Linker:
@classproperty
def backend(cls):
. . .Related: python/cpython#89519 There are caveats and hazards in complex cases, but they wouldn't come into play here. Another option would be to keep the property while adding a module-level function Is there any risk that the backend selection might become dependent on constructor args? If so, it would be better to use a freestanding function IMO. |
||
|
|
||
|
|
||
| # ============================================================================= | ||
|
|
||
|
leofang marked this conversation as resolved.
|
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.
In the meeting there was a discussion about making this a class property. It sounded like that's not entirely straightforward. I just asked here:
https://chatgpt.com/share/69f9637b-9280-83ea-ae35-12c7a18f9337
It seems indeed not straightforward.
My pitch in the meeting was: let's keep this simple and name this
@classmethodwhich_backendorget_backend(I'd pickwhich_backendbecause it seems more intuitive to me; likebashwhich cat).Apart from this pure naming question, this PR looks good and final to me.