-
Notifications
You must be signed in to change notification settings - Fork 355
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
feat: support aten.atan2.out converter #2829
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it's fine to have this warning. We just keep the same behavior as pytorch. A minor comment below, the others LGTM
from .harness import DispatchTestCase | ||
|
||
|
||
class TestAtan2OutConverter(DispatchTestCase): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you move the test class into the test_atan2_aten.py
considering they are just variants of atan2?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have moved the test class inside test_atan2_aten.py
, combining the test cases for aten.atan2.out
and aten.atan2.default
. Thank you for your useful comment.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are some changes that do not conform to Python style guidelines:
--- /home/runner/work/TensorRT/TensorRT/tests/py/dynamo/conversion/test_atan2_aten.py 2024-05-24 08:10:32.451612+00:00
+++ /home/runner/work/TensorRT/TensorRT/tests/py/dynamo/conversion/test_atan2_aten.py 2024-05-24 08:12:34.674439+00:00
@@ -125,10 +125,11 @@
self.run_test(
atan2(),
inputs,
)
+
class TestAtan2OutConverter(DispatchTestCase):
@parameterized.expand(
[
((10,), (5,), torch.float),
((10,), (10,), torch.float),
@@ -150,7 +151,8 @@
self.run_test(
atan2_out(),
inputs,
)
+
if __name__ == "__main__":
run_tests()
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are some changes that do not conform to Python style guidelines:
--- /home/runner/work/TensorRT/TensorRT/tests/py/dynamo/conversion/test_atan2_aten.py 2024-05-24 13:35:19.016148+00:00
+++ /home/runner/work/TensorRT/TensorRT/tests/py/dynamo/conversion/test_atan2_aten.py 2024-05-24 13:37:16.028715+00:00
@@ -125,10 +125,11 @@
self.run_test(
atan2(),
inputs,
)
+
class TestAtan2OutConverter(DispatchTestCase):
@parameterized.expand(
[
((10,), (5,), torch.float),
((10,), (10,), torch.float),
@@ -150,7 +151,8 @@
self.run_test(
atan2_out(),
inputs,
)
+
if __name__ == "__main__":
run_tests()
Description
The
aten.atan2.out
operation calculates the element-wise arctangent of two tensors and stores the results in a specified output tensor,out
. This does not alter the input tensors, meaning it isn't aninplace operation
in the traditional sense.However, I've encountered two issues:
First, when the shape of the input
out
tensor does not match theactual output shape
, PyTorch issues a warning but does not produce an error, and still outputs the correct results. I've attached the results of running the atan2.out operation in PyTorch below:Second, even though TensorRT displays an error message when above warning occurs, the test case still passes. I've attached the results of the executed test case below.
Fixes # (issue)
Type of change
Please delete options that are not relevant and/or add your own.
Checklist: