add TensorBase.is_complex #385
Conversation
khushi-411
requested review from
mruberry,
lantiga,
robieta,
t-vi and
carmocca
as code owners
| @@ -617,6 +617,7 @@ def _abs_torch(x: torch.Tensor | Number): | |||
| ), | |||
| ), | |||
| ) | |||
| elementwise_unary_ops.append(abs_opinfo) | |||
| @@ -1090,6 +1091,14 @@ def _abs_torch(x: torch.Tensor | Number): | |||
| ) | |||
| elementwise_unary_ops.append(floor_opinfo) | |||
|
|
|||
| is_complex_opinfo = OpInfo( | |||
There was a problem hiding this comment.
is_complex is more tensor property like is_cuda, so let's put its OpInfo with the other tensor properties:
| @@ -1102,6 +1102,11 @@ def ceil(a): | |||
| return clang.ceil(a) | |||
|
|
|||
|
|
|||
| @torchsymbol(torch.is_complex, is_method=True) | |||
There was a problem hiding this comment.
Let's not make this a symbol and let's put it next to is_cuda:
But then we need to two things (I think) to make it work properly:
- call
register_method() - update the map
_torch_to_thunder_function_map[torch.is_complex] = is_complex
Then I would expect the function and method to work as expected
There was a problem hiding this comment.
Thanks for taking this on, @khushi-411! Mike had me do the same in #429, if you want more detail here.
Note that if it's both a method and a function of the module root (i.e. torch.is_complex) then AFAICT one needs to update the map for both torch.is_complex and torch.Tensor.is_complex.
There was a problem hiding this comment.
Hi, @tfogal! Thanks very much for the heads-up. I'll look into the PR and keep you posted.
I'm currently travelling for a week, but I will complete this PR soon. I apologize for the delay.
|
Hey @khushi-411! I made some comments about how to add this as a non-symbol function and method. Let's see how that works! We haven't done a lot of that before, and other operators would benefit from a pattern for it, too |
Before submitting
What does this PR do?
Fixes #339
PR review
Anyone in the community is free to review the PR once the tests have passed.
If we didn't discuss your PR in Github issues there's a high chance it will not be merged.
Did you have fun?
Sure thing!!!