Support enabling cuml.accel with env var
#7046
Conversation
| add_subdirectory(cuml/experimental/linear_model) | ||
|
|
||
| # Include the pth file in the built wheel | ||
| install(FILES "_cuml_accel.pth" DESTINATION ".") |
There was a problem hiding this comment.
No idea if this is the proper way to do this, but it seems to work for wheels at least.
I haven't checked if edit: now checked, this seems to be correct too._cuml_accel.pth is included in an sdist yet.
betatim
left a comment
There was a problem hiding this comment.
Not sure about the CMakeLists part, I'll let someone else comment on that.
The rest looks fine to me.
For my education: is there an example program/application where you can not use -p or the programmatic way of enabling this?
csadorf
left a comment
There was a problem hiding this comment.
I don't have immediate concerns with the implementation, but I'd like to have a bit more motivation for this. Specifically, I'm a bit worried that this sets false expectations.
Specifically, enabling via environment variable means that we don't fail if cuml is not installed at all.
|
The primary motivation for this was the follow-up PR (#7047). The mechanism added here is the cleanest and most robust way of supporting that use case. I figured exposing it to a user would be harmless and increase the flexibility of the tool. Environment variables feel even more zero code change since they don't require changing the script/notebook or command used to execute them. For prior art, the mechanism for If I added a line in the docs noting your worry would that be sufficient? |
Yes. It should contextualize that this approach – unlike the others – will not fail if cuML is not installed and that it is most suitable when you don't want or cannot change the entrypoint. I think by adding yet another alternative way of enabling zero-code change we need to provide the littlest bit of guidance on when to use which approach. |
This adds support for enabling `cuml.accel` by setting the `CUML_ACCEL_ENABLED` environment variable to `1` or `true` (case insensitive). This can be useful for enabling acceleration in applications where you cannot invoke the CLI or call `cuml.accel.install()` programmatically.
398b365 to
10bcb73
Compare
|
Done |
As mentioned above, the primary motivation for this was propagating If nothing else, it makes it easier to have a programmatic switch on whether the accelerator is turned on or not in a deployment scenario since you just have to change the value of an environment variable (in e.g. a k8s pod spec) rather than change the executable line. |
betatim
left a comment
There was a problem hiding this comment.
Thanks for the explanations and adding context/motivation. Still looks good to me
|
/merge |
This makes use of the `CUML_ACCEL_ENABLED` environment variable to ensure if cuml.accel is enabled in a parent process then it's enabled (by default) in a child process. This makes `cuml.accel` usage more transparent with common python multiprocessing solutions (`multiprocessing`/`joblib`/`dask`). Things _just work_. Currently on top of #7046. Note that this use case was the primary motivation for #7046, though I can see other reasons why an environment variable might be nice too. Authors: - Jim Crist-Harif (https://github.com/jcrist) - Simon Adorf (https://github.com/csadorf) Approvers: - Simon Adorf (https://github.com/csadorf) URL: #7047
This adds support for enabling
cuml.accelby setting theCUML_ACCEL_ENABLEDenvironment variable to1ortrue(case insensitive). This can be useful for enabling acceleration in applications where you cannot invoke the CLI or callcuml.accel.install()programmatically.