Skip to content

Conversation

apbose
Copy link
Collaborator

@apbose apbose commented May 24, 2024

Description

Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context. List any dependencies that are required for this change.

Fixes # (issue)

Type of change

Please delete options that are not relevant and/or add your own.

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

Checklist:

  • My code follows the style guidelines of this project (You can use the linters)
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas and hacks
  • I have made corresponding changes to the documentation
  • I have added tests to verify my fix or my feature
  • New and existing unit tests pass locally with my changes
  • I have added the relevant labels to my PR in so that relevant reviewers are notified

@apbose apbose self-assigned this May 24, 2024
@apbose apbose marked this pull request as draft May 24, 2024 00:22
@github-actions github-actions bot added component: lowering Issues re: The lowering / preprocessing passes component: api [Python] Issues re: Python API component: dynamo Issues relating to the `torch.compile` or `torch._dynamo.export` paths labels May 24, 2024
@github-actions github-actions bot added the component: tests Issues re: Tests label Jun 6, 2024
@apbose apbose marked this pull request as ready for review June 6, 2024 22:46
@apbose apbose requested review from zewenli98 and chohk88 June 6, 2024 22:46
@apbose apbose force-pushed the empty_strided_decomp branch from 41f0324 to 1360961 Compare June 6, 2024 22:53
Copy link

@github-actions github-actions bot left a 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/lowering/test_decompositions.py	2024-06-06 22:53:37.987181+00:00
+++ /home/runner/work/TensorRT/TensorRT/tests/py/dynamo/lowering/test_decompositions.py	2024-06-06 22:55:54.420669+00:00
@@ -869,29 +869,29 @@
            DECIMALS_OF_AGREEMENT,
            f"Select_scatter TRT outputs don't match with the original model.",
        )

        empty_ops = [
-        (
-            "empty_stride_one_dimension_firstcase",
-            [5, 5],
-            [1, 2],
-            None,
-        ),
-        (
-            "empty_stride_two_dimension_secondcase",
-            [5, 5],
-            [2, 2],
-            None,
-        ),
-        (
-            "empty_three_dimension",
-            [8, 8, 8],
-            [1, 2, 3],
-            torch.int32,
-        ),
-    ]
+            (
+                "empty_stride_one_dimension_firstcase",
+                [5, 5],
+                [1, 2],
+                None,
+            ),
+            (
+                "empty_stride_two_dimension_secondcase",
+                [5, 5],
+                [2, 2],
+                None,
+            ),
+            (
+                "empty_three_dimension",
+                [8, 8, 8],
+                [1, 2, 3],
+                torch.int32,
+            ),
+        ]

    @parameterized.expand(
        [(empty_op[0], empty_op[1], empty_op[2], empty_op[3]) for empty_op in empty_ops]
    )
    def test_empty_stride(self, _, shape_or_input, stride, data_type):

@apbose apbose marked this pull request as draft June 10, 2024 20:47
Copy link
Collaborator

@zewenli98 zewenli98 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall LGTM, just need a minor fix

from torch.testing._internal.common_utils import TestCase, run_tests
from parameterized import parameterized
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

duplicated as line 3

@apbose apbose force-pushed the empty_strided_decomp branch from 1360961 to aeae9b3 Compare June 11, 2024 17:27
@apbose apbose marked this pull request as ready for review June 11, 2024 17:28
@apbose apbose force-pushed the empty_strided_decomp branch 2 times, most recently from ec15e77 to e296688 Compare June 14, 2024 21:22
adding test cases and correcting empty__stride decomposition
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla signed component: api [Python] Issues re: Python API component: dynamo Issues relating to the `torch.compile` or `torch._dynamo.export` paths component: lowering Issues re: The lowering / preprocessing passes component: tests Issues re: Tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants