🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/157859

• 📄 Preview Python docs built from this PR
• 📄 Preview C++ docs built from this PR
• ❓ Need help or want to give feedback on the CI? Visit the bot commands wiki

Note: Links to docs will display an error until the docs builds have been completed.

✅ You can merge normally! (1 Unrelated Failure)

As of commit 7464a8e with merge base c58e096 ():

This comment was automatically generated by Dr. CI and updates every 15 minutes.

Hmm... Specializing PrivateUse1HooksInterface and DeviceGuardImplInterface for PrivateUse1 is not a good idea directly . If you really want to implement this functionality based on pure Python, why not consider exposing these two classes to the Python side, and then let users inherit and implement the specific details of these two classes on the Python side? In addition, can __torch_function__ and __torch_dispatch__ basically achieve what you want?

Hmm... Specializing PrivateUse1HooksInterface and DeviceGuardImplInterface for PrivateUse1 is not a good idea directly . If you really want to implement this functionality based on pure Python, why not consider exposing these two classes to the Python side, and then let users inherit and implement the specific details of these two classes on the Python side?

This is indeed a better idea. I'll try to do that. Initially I picked the least intrusive thing (everything optional, and do as little as it can).

In addition, can __torch_function__ and __torch_dispatch__ basically achieve what you want?

Indeed it does basically (https://dev-discuss.pytorch.org/t/embrace-tensor-subclass-as-a-python-device-registration-api/2771); and it is what I am using right now. There are places where it interacts poorly with torch, including the 2 issues that this PR fixes; as well as AMP integration.

I have an idea. Apology if there are any problems.

Can we directly export PrivateUse1HooksInterface and DeviceGuardImplInterface to Python through pybind11 and trampoline class (the code can be put in torch/csrc/Modules.cpp, because this is the logic of binding between Python and C++), and then provide an API to register instances of the above two classes from the Python side;

In this way, users can integrate the classes on the Python side to implement the corresponding interfaces, and can also easily know which interfaces need to be implemented; whats more, there is no need to store PythonPrivateUse1DeviceGuardandPythonPrivateUse1HooksInterface` in PyTorch.

Of course, it is also possible to forward all operations from C++ back to Python like the current PR implementation, but we'd better provide a base class consistent with C++ for users to inherit, so that users can easily understand which interfaces may need to be implemented.

@pytorchbot label "release notes: python_frontend"

Hi @fffrog @albanD :

Made few changes as per discussion:

1. Made the Hook and Device class C++ classes exposed to python with trampoline as suggested. To make this work, I followed instructions on https://pybind11.readthedocs.io/en/stable/advanced/classes.html; and, had to update the version of pybind11 in third_party/ because the old version did not have the header <pybind11/trampoline_self_life_support.h>. LMK if this is OK or not.
2. Moved changes to a new directory torch/csrc/pytorch_custom_backend; as based on

   pytorch/torch/csrc/README.md

   Line 4 in f6c89c1

   with Python. This is in contrast to lib, which contains the Torch

   it seems more desired to have python binding stuff there than C10.
3. Changes in build_variables.bzl to make the file builds.

Let me know how it looks, also please start CI to see if any tests fails; I don't have the permission to do that.

Thanks!

Thank you.

Made the Hook and Device class C++ classes exposed to python with trampoline as suggested. To make this work, I followed instructions on https://pybind11.readthedocs.io/en/stable/advanced/classes.html; and, had to update the version of pybind11 in third_party/ because the old version did not have the header <pybind11/trampoline_self_life_support.h>. LMK if this is OK or not.

The upgrade of pybind11 needs to be done very carefully, so we need to find other ways to solve this problem.

Back to the problem itself, we do need to pay attention to the variable life cycle in C++ and Python. In my opinion, using the holder_type of shared_ptr should be a feasible method.

The upgrade of pybind11 needs to be done very carefully,

Indeed.

so we need to find other ways to solve this problem.

I can also make a separate PR to just update pybind11 just to be careful. would you think that is a good idea?

Back to the problem itself, we do need to pay attention to the variable life cycle in C++ and Python. In my opinion, using the holder_type of shared_ptr should be a feasible method.

would you describe what is the life cycle issue? I thought the reason of why we need the header is because that is how to get definitions of PYBIND11_OVERRIDE_PURE needed for setting the trampoline.

would you describe what is the life cycle issue? I thought the reason of why we need the header is because that is how to get definitions of PYBIND11_OVERRIDE_PURE needed for setting the trampoline.

The definition of PYBIND11_OVERRIDE_PURE or PYBIND11_OVERRIDE is in the pybind11/pybind11 file, you can search for it in the PyTorch codebase.

Also, it seems to me that py::trampoline_self_life_support and py::smart_holder were introduced in v3.0 to solve the problem of variable lifetime. The user will inherit torch._C.PrivateUse1Hooks and register the custom class instance into C++ through register_python_privateuseone_hook, and other modules in C++ will use this C++ class instance throughout the entire process, so we need to always keep it active and cannot release it for any reason, including Python variables being released because they are out of scope, otherwise core dump may occur due to dangling pointers.

@pytorchbot merge

Merge started

Your change will be merged once all checks pass (ETA 0-4 Hours).

Learn more about merging in the wiki.

Questions? Feedback? Please reach out to the PyTorch DevX Team

@pytorchbot revert -m "introduce linting errors https://github.com/pytorch/pytorch/actions/runs/18123993236/job/51574878473" -c nosignal

@pytorchbot successfully started a revert job. Check the current status here.
Questions? Feedback? Please reach out to the PyTorch DevX Team

@qihqi your PR has been successfully reverted.

Hi @jeanschmidt would you point out what linting error you were seeing? I am looking at https://hud.pytorch.org/pr/157859 and couldnt find any

Relevant to point out, the issue introduced seems to come from the job lintrunner-noclang / linux-job

>>> Lint for torch/_C/__init__.pyi:

  Warning (RUFF) W292
    No newline at end of file.
    See [https://beta.ruff.rs/docs/rules/.](https://beta.ruff.rs/docs/rules/)
    
    To disable, use `  # noqa: W292`

        12933  |        arg_types: str,
        12934  |        args: tuple[Any, ...],
        12935  |        stream: _int,
    >>> 12936  |    ) -> None: ...
  Warning (RUFF) format
    Run `lintrunner -a` to apply this patch.

    You can run `lintrunner -a` to apply this patch.

    12933  12933 |         arg_types: str,
    12934  12934 |         args: tuple[Any, ...],
    12935  12935 |         stream: _int,
    12935        |-    ) -> None: ...
           12936 |+    ) -> None: ...

  Warning (PYFMT) format
    Run `lintrunner -a` to apply this patch.

    You can run `lintrunner -a` to apply this patch.

    12933  12933 |         arg_types: str,
    12934  12934 |         args: tuple[Any, ...],
    12935  12935 |         stream: _int,
    12935        |-    ) -> None: ...
           12936 |+    ) -> None: ...

+ echo ''

@pytorchbot merge

Merge started

Your change will be merged once all checks pass (ETA 0-4 Hours).

Learn more about merging in the wiki.

Questions? Feedback? Please reach out to the PyTorch DevX Team

FYI the test added here has been failing in periodic CI since it was added in october: https://hud.pytorch.org/failure?name=periodic%20%2F%20linux-jammy-cuda12.8-py3.10-gcc11-debug%20%2F%20test%20(default%2C%201%2C%207%2C%20linux.g6.4xlarge.experimental.nvidia.gpu%2C%20oncall%3Adebug-build)&jobName=undefined&failureCaptures=test%2Ftest_privateuseone_python_backend.py%3A%3APrivateUse1BackendTest%3A%3Atest_backend_simple

We should fix this! (not super urgent).

We should fix this! (not super urgent).

So sorry for this, i will fix it