Make Linker.backend a classmethod#1910
Conversation
|
dbe6176 to
215418a
Compare
leofang
left a comment
There was a problem hiding this comment.
Thanks, Phillip! Implementation-wise LGTM. Left a few suggestions.
Allows querying the linking backend without constructing a Linker instance — useful for dispatching on input format (PTX vs. LTOIR) before linking. Updates existing call sites (Program init, test_linker) to use the new invocation form Linker.backend().
Covers classmethod invocation (Linker.backend() without an instance), memoisation flag handling, probe-on-first-use, and non-property attribute semantics.
Breaking change: Linker.backend is now a classmethod, so call sites must use parens: Linker.backend().
215418a to
12d291b
Compare
leofang
left a comment
There was a problem hiding this comment.
Given that we just recently went to the opposite direction (converting some methods to properties, #1945 & #1986), it'd be better to have @mdboom or @Andy-Jost to chime in from the design consistency perspective. It might be possible that we need to re-evaluate if #714 is really necessary.
| @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" |
There was a problem hiding this comment.
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 get_backend().
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.
Summary
Linker.backendfrom an instance property to a classmethod so callers can query the linking backend without constructing aLinker_decide_nvjitlink_or_driver()(existing memoised probe) and maps the return value to"nvJitLink"or"driver"_program.pyxand existing test assertion to use parensDevice()/cuInitrequired)0.8.0-notes.rstdocumenting the breaking changeBreaking change:
linker.backend(attribute access) now returns a bound method, not a string. All call sites must useLinker.backend(). Only 2 in-repo call sites affected; numba-cuda (the requester) will adoptLinker.backend()from day one.Test plan
test_linker_initpasses with updated paren callCloses #714
🤖 Generated with Claude Code