Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

torch native functions cannot be used with inspect.signature #28233

Open
ngoldbaum opened this issue Oct 17, 2019 · 6 comments
Open

torch native functions cannot be used with inspect.signature #28233

ngoldbaum opened this issue Oct 17, 2019 · 6 comments
Labels
module: language binding support for language bindings, including languages that aren't currently supported module: pybind Related to our Python bindings / interactions with other Python libraries triaged This issue has been looked at a team member, and triaged and prioritized into an appropriate module

Comments

@ngoldbaum
Copy link
Contributor

ngoldbaum commented Oct 17, 2019

🚀 Feature

It would be nice if native functions were annotated with the necessary metadata to allow runtime introspection via the inspect module.

Motivation

Right now using e.g. inspect.signature with a function defined natively produces a ValueError:

>>> import inspect
>>> import torch
>>> inspect.signature(torch.mm)
*** ValueError: no signature found for builtin <built-in method mm of type object at 0x7ff8961cc940>

Pitch

It would be nice if the native function generation facilities in the pytorch build could create the function objects with the necessary metadata.

Cython is able to do this, so it's definitely possible:

>>> import cython
>>> import inspect
>>> scope = cython.inline("""def f(a,*args,b=False): pass """)
>>> inspect.signature(scope['f'])
<Signature (a, *args, b=False)>

Unfortunately I don't know enough about how cython does this or how python builtins can get annotated with the necessary metadata to point to further resources for doing this in pytorch.

Alternatives

Another way to do this would be for pytorch to make a public API that provides this information, since it's available in native_functions.yaml at build time. I think this is less nice since it should be possible to integrate with python's native introspection facilities.

@smessmer smessmer added module: language binding support for language bindings, including languages that aren't currently supported module: pybind Related to our Python bindings / interactions with other Python libraries triaged This issue has been looked at a team member, and triaged and prioritized into an appropriate module labels Oct 17, 2019
@smessmer
Copy link
Contributor

This is a good idea. I would review a PR implementing this.

facebook-github-bot pushed a commit that referenced this issue Mar 3, 2020
Summary:
Fixes #33182

This adds private API functions that developers of types that implement `__torch_function__` can use to ensure full coverage of the subset of the PyTorch API that can be overrided.

I've refactored some of the code in the tests into a new `torch._overrides.get_overridable_functions` function. I've also changed `TENSOR_LIKE_TORCH_OVERRIDES` into `torch._overrides.get_testing_overrides` and `IGNORED_TORCH_FUNCTIONS` into `torch._overrides.get_ignored_functions`. Making these two static global variables in the tests into functions should allow rewriting their implementation to construct their return values instead of just statically defining the return value as is done here. Currently that is blocked on not being able to inspect function signatures of compiled kernels in PyTorch (see #28233). See the docs I've added for usage examples of these new functions. I also refactored the existing override tests to make use of these new functions, which should be a good forcing function to make sure they're kept up-to-date.

Finally, while working on this I discovered that `TestTorchFunctionOverrides.test_mean` and `TestTorchFunctionOverrides.test_mm` weren't ever being run because they were getting clobbered by the other dynamically generated override tests. I fixed that by renaming the tests and then fixing the actual test code. I've verified that all the subclassing semantics is correct and that the updated test answers are correct. I'm happy to put the fixes to the existing tests in as a separate pull request if that would be easier to review.

ping cpuhrsch since the feature request originally came from them.
Pull Request resolved: #33791

Differential Revision: D20195053

Pulled By: cpuhrsch

fbshipit-source-id: 1585f4e405f5223932b410eae03a288dc8eb627e
ttumiel pushed a commit to ttumiel/pytorch that referenced this issue Mar 4, 2020
…rch#33791)

Summary:
Fixes pytorch#33182

This adds private API functions that developers of types that implement `__torch_function__` can use to ensure full coverage of the subset of the PyTorch API that can be overrided.

I've refactored some of the code in the tests into a new `torch._overrides.get_overridable_functions` function. I've also changed `TENSOR_LIKE_TORCH_OVERRIDES` into `torch._overrides.get_testing_overrides` and `IGNORED_TORCH_FUNCTIONS` into `torch._overrides.get_ignored_functions`. Making these two static global variables in the tests into functions should allow rewriting their implementation to construct their return values instead of just statically defining the return value as is done here. Currently that is blocked on not being able to inspect function signatures of compiled kernels in PyTorch (see pytorch#28233). See the docs I've added for usage examples of these new functions. I also refactored the existing override tests to make use of these new functions, which should be a good forcing function to make sure they're kept up-to-date.

Finally, while working on this I discovered that `TestTorchFunctionOverrides.test_mean` and `TestTorchFunctionOverrides.test_mm` weren't ever being run because they were getting clobbered by the other dynamically generated override tests. I fixed that by renaming the tests and then fixing the actual test code. I've verified that all the subclassing semantics is correct and that the updated test answers are correct. I'm happy to put the fixes to the existing tests in as a separate pull request if that would be easier to review.

ping cpuhrsch since the feature request originally came from them.
Pull Request resolved: pytorch#33791

Differential Revision: D20195053

Pulled By: cpuhrsch

fbshipit-source-id: 1585f4e405f5223932b410eae03a288dc8eb627e
@albanD
Copy link
Collaborator

albanD commented Apr 25, 2022

I am not sure if this is possible to do though?
A lot of our functions are designed based on overloads. For example add:

@overload
def add(input: Union[Tensor, Number], other: Union[Tensor, Number], *, alpha: Optional[Number]=1, out: Optional[Tensor]=None) -> Tensor: ...
@overload
def add(self: Tensor, alpha: Number, other: Tensor) -> Tensor: ...
@overload
def add(self: Tensor, alpha: Number, other: Tensor, *, out: Tensor) -> Tensor: ...

So there isn't a single signature we can generate for this op.

@stuaxo
Copy link

stuaxo commented Dec 14, 2022

I think you can build signatures that are build up of more than one signature, using Union

Looking at the code for @override, it keeps a registry you can lookup by module + qualified name, and line # -

https://github.com/python/cpython/blob/5c19050546e3e37a8889a0baa2954e1444e803d3/Lib/typing.py#L2550

So it may be possible to build a Union from all the signatures collected by override.

@asmeurer
Copy link
Collaborator

Would it at least be possible to include all those overloads in the documentation? Right now help(torch.add) and the online docs show add(input, other, *, alpha=1, out=None) -> Tensor, which is just one of the above listed overloads. These are shown in error messages, but it would be helpful to include them in the docs as well.

@stuaxo
Copy link

stuaxo commented Jan 12, 2023

@asmeurer that possible, and worth adding as another issue.

__doc__ can be created when the new function is. If it's still difficult, I've noticed __doc__ can be an fstring.

@blueardour
Copy link

any update?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
module: language binding support for language bindings, including languages that aren't currently supported module: pybind Related to our Python bindings / interactions with other Python libraries triaged This issue has been looked at a team member, and triaged and prioritized into an appropriate module
Projects
None yet
Development

No branches or pull requests

6 participants