Support out-of-tree plugins autoloading#1151
Conversation
This commit adds support of out-of-tree plugins autoloading using python package entry point specification: * https://packaging.python.org/en/latest/specifications/entry-points/#entry-points * https://packaging.python.org/en/latest/guides/creating-and-discovering-plugins/#using-package-metadata Out-of-tree plugins must register entry points as `torchcodec.backends`: ``` [project.entry-points.'torchcodec.backends'] device_backend = 'torchcodec_plugin:load_plugin' ``` Torchcodec will automatically load plugins if discovered. Loading can be explicitly suppressed with `TORCHCODEC_DEVICE_BACKEND_AUTOLOAD=1` environment variable. The same approach is being used to load PyTorch device backends. See: * pytorch/pytorch#127074 Signed-off-by: Dmitry Rogozhkin <dmitry.v.rogozhkin@intel.com>
Requires: meta-pytorch/torchcodec#1151 Signed-off-by: Dmitry Rogozhkin <dmitry.v.rogozhkin@intel.com>
|
Thanks for the PR @dvrogozh . Just to make sure I understand correctly, the main goal of this PR is to go from: import torchcodec
import some_torchcodec_pluginto: import torchcodec # automatically loads some_torchcodec_plugin if it was installedIs my understanding correct? |
|
@NicolasHug, yes, your understanding is correct. |
|
Thanks for confirming @dvrogozh . I'm still in the process to form an opinion on this, so please bear with me. My first reaction was pretty much the same as Ed's (pytorch/pytorch#122468 (comment)): an extra import isn't the end of the world. I get the argument that some scripts are device agnostic and that the device is passed as a CLI parameter, in which case having to do an extra import is a problem. Whether that's really an issue in practice for torchcodec, I don't know yet. Quick Qs which will help me form a better opinion:
Basically, I just want to make sure that users will be using a new out-of-tree backend only when they explicitly request it, either by passing |
Yes, that is right. Users have to pass the device explicitly. For example, that's what Huggingface Transformers are doing:
As of today TorchCodec handles torchcodec/src/torchcodec/decoders/_video_decoder.py Lines 223 to 224 in 48844c8 User can explicitly override the default device by calling |
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/meta-pytorch/torchcodec/1151
Note: Links to docs will display an error until the docs builds have been completed. ✅ No FailuresAs of commit 8fc6410 with merge base 04a1e34 ( This comment was automatically generated by Dr. CI and updates every 15 minutes. |
NicolasHug
left a comment
There was a problem hiding this comment.
LGTM, I was a bit on the fence with this but the argument about passing --device from the shell CLI makes sense.
I made a few simplification to the tests.
| ```python | ||
| import torchcodec_xpu | ||
| ``` | ||
|
|
There was a problem hiding this comment.
@dvrogozh can you confirm we can remove this if this PR is merged?
There was a problem hiding this comment.
The import torchcodec_xpu corresponds to the current released versions of torchcodec and torchcodec xpu plugin. Merging just this PR won't make autoloading work for XPU as respective change needs to be done on xpu plugin side as well (to register it for autoloading). Overall we will need to make new torchcodec and tochcodec xpu plugin releases with these changes to make everything work and formally remove this line.
I think that this should be OK to drop the line right now considering that this is development repository. In this way we will be heading to make instruction correct on the next approaching releases of torchcodec and xpu plugin. From my side I can commit for us doing the xpu plugin release in a short follow up after torchcodec one - 1-2 days timeframe.
This commit adds support of out-of-tree plugins autoloading using python package entry point specification:
Out-of-tree plugins must register entry points as
torchcodec.backends:Torchcodec will automatically load plugins if discovered. Loading can be explicitly suppressed with
TORCHCODEC_DEVICE_BACKEND_AUTOLOAD=1environment variable.The same approach is being used to load PyTorch device backends. See:
Here is a PR for XPU plugin which will enable autoloading if torchcodec PR will get merged:
CC: @scotts @NicolasHug