CuPy noticed the error type change for `load_nvidia_dynamic_lib("nccl")` (windows)

[seberg]

Probably better to open an issue: I don't have a very strong opinion on this, but it would be nice to know where to go (i.e. if to catch both/all errors in CuPy or just blocklist the one version in CI).

CuPy notice this change in CI (if probably just cupy.show_runtime() breaking on windows). (Checking for nccl.)

I.e. the CuPy code currently assumed that load_nvidia_dynamic_lib always raises DynamicLibNotFoundError. It could be fixed on either side, but maybe it makes sense to just keep raising DynamicLibNotFoundError (with the new message), so that downstream try/except is unaffected?

(Although, I am not sure if it may be nice to be able to distinguish the two errors.)

CC @leofang

Originally posted by @seberg in #1602 (comment)

[kkraus14]

cc @rwgk

I believe this is intended since the previous was undefined and unsupported behavior

[rwgk]

cc @rwgk

I believe this is intended since the previous was undefined and unsupported behavior

Yes, but TBH I didn't think carefully about the case that a libname is supported on one platform but not another (in this case supported under Linux, but not Windows).

I.e. sorry for the breakage @seberg

Thinking out loud:

Ideally we'd cleanly distinguish:

• libname unknown / not supported on any platform (ValueError seems fine for that case, or maybe hypothetical DynamicLibUnknown)
• libname not supported on this platform (hypothetical DynamicLibNotAvailable)
• libname known and supported, but not found (existing DynamicLibNotFoundError )

In retrospect, I think we should have done it this way:

class DynamicLibNotFoundError(RuntimeError): ...  # established

class DynamicLibUnknownError(DynamicLibNotFoundError): ...  # new

class DynamicLibNotAvailableError(DynamicLibNotFoundError): ...  # new

That would be backward compatible. And easy.

Should we yank the 1.3.5 release (and at the same time release 1.3.6)? It's still less than 24 hours since it was made.

[rwgk]

@cpcloud for visibility

[kkraus14]

Should we yank the 1.3.5 release (and at the same time release 1.3.6)? It's still less than 24 hours since it was made.

No, we shouldn't yank the release just in case someone has pinned it already, but we can call out in the release note that the error type we were returning was a bug and that it has been corrected in 1.3.6 or 1.4.0

[rwgk]

Should we yank the 1.3.5 release (and at the same time release 1.3.6)? It's still less than 24 hours since it was made.

No, we shouldn't yank the release just in case someone has pinned it already, but we can call out in the release note that the error type we were returning was a bug and that it has been corrected in 1.3.6 or 1.4.0

OK, thanks! I'll change the release notes in #1688 accordingly.

[rwgk]

@seberg FYI, the pathfinder 1.4.0 release with the fix is complete now:

https://github.com/NVIDIA/cuda-python/releases/tag/cuda-pathfinder-v1.4.0