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
scatter_add_decomposition #2740
base: main
Are you sure you want to change the base?
Conversation
a49c420
to
e8c7b50
Compare
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/py/torch_tensorrt/dynamo/lowering/_decompositions.py 2024-05-14 21:04:24.249027+00:00
+++ /home/runner/work/TensorRT/TensorRT/py/torch_tensorrt/dynamo/lowering/_decompositions.py 2024-05-14 21:06:14.833958+00:00
@@ -181,13 +181,16 @@
input_tensor: torch.Tensor,
src_tensor: torch.Tensor,
dim: int,
index: torch.Tensor,
) -> torch.Tensor:
- input_tensor_to_add = torch.scatter(torch.empty_like(input_tensor), dim, index, src_tensor)
+ input_tensor_to_add = torch.scatter(
+ torch.empty_like(input_tensor), dim, index, src_tensor
+ )
scatter_add_tensor = torch.add(input_tensor, input_tensor_to_add.cuda())
return scatter_add_tensor
+
def get_decompositions(
enable_experimental_decompositions: bool = False,
) -> Dict[OpOverload, Callable[[Any], Any]]:
if enable_experimental_decompositions:
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/py/torch_tensorrt/dynamo/lowering/_decompositions.py 2024-05-14 21:04:33.408462+00:00
+++ /home/runner/work/TensorRT/TensorRT/py/torch_tensorrt/dynamo/lowering/_decompositions.py 2024-05-14 21:06:24.734368+00:00
@@ -181,13 +181,16 @@
input_tensor: torch.Tensor,
src_tensor: torch.Tensor,
dim: int,
index: torch.Tensor,
) -> torch.Tensor:
- input_tensor_to_add = torch.scatter(torch.empty_like(input_tensor), dim, index, src_tensor)
+ input_tensor_to_add = torch.scatter(
+ torch.empty_like(input_tensor), dim, index, src_tensor
+ )
scatter_add_tensor = torch.add(input_tensor, input_tensor_to_add.cuda())
return scatter_add_tensor
+
def get_decompositions(
enable_experimental_decompositions: bool = False,
) -> Dict[OpOverload, Callable[[Any], Any]]:
if enable_experimental_decompositions:
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/py/torch_tensorrt/dynamo/lowering/_decompositions.py 2024-05-14 21:01:28.751182+00:00
+++ /home/runner/work/TensorRT/TensorRT/py/torch_tensorrt/dynamo/lowering/_decompositions.py 2024-05-14 21:09:51.626381+00:00
@@ -181,13 +181,16 @@
input_tensor: torch.Tensor,
src_tensor: torch.Tensor,
dim: int,
index: torch.Tensor,
) -> torch.Tensor:
- input_tensor_to_add = torch.scatter(torch.empty_like(input_tensor), dim, index, src_tensor)
+ input_tensor_to_add = torch.scatter(
+ torch.empty_like(input_tensor), dim, index, src_tensor
+ )
scatter_add_tensor = torch.add(input_tensor, input_tensor_to_add.cuda())
return scatter_add_tensor
+
def get_decompositions(
enable_experimental_decompositions: bool = False,
) -> Dict[OpOverload, Callable[[Any], Any]]:
if enable_experimental_decompositions:
dim: int, | ||
index: torch.Tensor, | ||
) -> torch.Tensor: | ||
input_tensor_to_add = torch.scatter(torch.empty_like(input_tensor), dim, index, src_tensor) |
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.
Could torch.empty_like(input_tensor)
instead be just input_tensor
, or is it required to be empty?
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.
Yes it would be required. The logic is like we would apply scatter with the indices to an empty tensor and then add it to the input tensor.
index: torch.Tensor, | ||
) -> torch.Tensor: | ||
input_tensor_to_add = torch.scatter(torch.empty_like(input_tensor), dim, index, src_tensor) | ||
scatter_add_tensor = torch.add(input_tensor, input_tensor_to_add.cuda()) |
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.
Without .cuda()
does the decomposition fail?
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.
Yes in this case it complains that the two tensors are on different devices.
@gs-olive though the test cases pass, I don't think that the decomposition is taking place.
Is this due to torch.unique? |
No description provided.