fix: Issue in non-Tensor Input Resolution #1617

gs-olive · 2023-01-25T02:27:39Z

Description

In certain TensorRT-executed blocks, multiple non-Tensor inputs originate from the same parent Tensor in another Torch block, so a single pass of resolveTRTNonTensorInputs does not resolve all non-Tensor inputs, causing failures at compile-time
Issue was determined to trace to the use of prim::Loop in the TensorRT block and is fixed in PR fix: fix the prim::Loop fallback issue #1691
Add function to SegmentedBlock to get the ID of a block, so re-inserted blocks retain the same ID as the previous block being replaced
Add regression test case to elicit behavior

Failure Case

Before Non-Tensor Input Resolution

DEBUG: [Torch-TensorRT - Debug Build] - Finalizing in progress TensorRT block
DEBUG: [Torch-TensorRT - Debug Build] - Segment Block @2:
    Target: TensorRT

    Graph: graph(%1 : int,
      %9 : int[],
      %12 : Tensor):

After 1 Round of Non-Tensor Input Resolution

GRAPH: [Torch-TensorRT - Debug Build] - Running shape analysis on block Segment Block @2:
    Target: TensorRT

    Graph: graph(%1 : int[],
      %3 : Tensor):

After 2 Rounds of Non-Tensor Input Resolution

INFO: [Torch-TensorRT - Debug Build] - Segment Block @2:
    Target: TensorRT

    Graph: graph(%1 : Tensor):

Fixes #1612
Related to: #1613

Type of change

Bug fix (non-breaking change which fixes an issue)

Checklist:

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

github-actions

Code conforms to C++ style guidelines

github-actions

Code conforms to Python style guidelines

github-actions

Code conforms to Python style guidelines

github-actions

Code conforms to C++ style guidelines

github-actions

Code conforms to C++ style guidelines

github-actions

Code conforms to Python style guidelines

peri044 · 2023-02-03T00:46:02Z

Expected this PR to make it into 23.03 release

bowang007 · 2023-02-21T21:03:31Z

This does not make sense in fact.
Firstly, even if 2 non-Tensor inputs come from a same node, that node will be added into the segmented block. There is no need to do it for multiple passes.
Secondly, I guess why it fails for a single pass is because that there is a Loop block in the TensorRT segment. This could introduce undefined behaviors. The real issue I think here is the Loop block, it shouldn't be in TensorRT segment at all. In our last code refactoring for partitioning, we set all nodes aggressively to be run in TensorRT, this introduces some issue like LOOP nodes default to be run in TensorRT, this could result in some issues like this.

- In certain TensorRT-executed blocks, multiple non-Tensor inputs originate from the same parent Tensor in another Torch block, so a single pass of `resolveTRTNonTensorInputs` does not resolve all non-Tensor inputs, causing failures at compile-time - Add do-while loop to ensure all input dependencies are resolved by running dependency-resolution algorithm multiple times, if necessary - Add function to `SegmentedBlock` to get the ID of a block, so re-inserted blocks retain the same ID as the previous block being replaced

- Added test case in partitioning to elicit bug arising from multiple nonTensor inputs to TensorRT block

…and test case

github-actions

Code conforms to C++ style guidelines

github-actions

Code conforms to Python style guidelines

github-actions

Code conforms to C++ style guidelines

github-actions

Code conforms to Python style guidelines

gs-olive · 2023-02-22T04:10:42Z

core/partitioning/partitioning.cpp

+        cur_partitioned_block[i] =
+            SegmentedBlock(cur_partitioned_block[i].get_id(), SegmentedBlock::kTensorRT, dependency_nodes);


Added to ensure the block IDs of segmented blocks stay in order despite resolution of non-Tensor inputs.

bowang007

LGTM, please squash your commits then merge.

facebook-github-bot added the cla signed label Jan 25, 2023

github-actions bot added component: core Issues re: The core compiler component: partitioning labels Jan 25, 2023

github-actions bot requested a review from narendasan January 25, 2023 02:27

gs-olive requested review from bowang007 and removed request for narendasan January 25, 2023 02:28

github-actions bot approved these changes Jan 25, 2023

View reviewed changes

gs-olive self-assigned this Jan 25, 2023

github-actions bot added the component: tests Issues re: Tests label Jan 27, 2023

github-actions bot approved these changes Jan 27, 2023

View reviewed changes

< 10000 div class="d-flex flex-auto">

fix: fix the prim::Loop fallback issue

1c11c36

bowang007 mentioned this pull request Feb 22, 2023

fix: fix the prim::Loop fallback issue #1691

Merged

7 tasks

bowang007 and others added 4 commits February 21, 2023 18:43

chore: fix typo

6f3ec49

fix: Added regression test case to elicit bug

5451bc4

- Added test case in partitioning to elicit bug arising from multiple nonTensor inputs to TensorRT block

Roll back partitioning nonTensor input changes, keep Block ID change …

ac715b0

…and test case

gs-olive force-pushed the resolve_nontensor_inputs_bugfix branch from 62f59c6 to ac715b0 Compare February 22, 2023 04:04

github-actions bot approved these changes Feb 22, 2023

View reviewed changes

gs-olive changed the base branch from main to fix_loop_fallback February 22, 2023 04:06

gs-olive requested review from bowang007 and removed request for bowang007 February 22, 2023 04:09

github-actions bot approved these changes Feb 22, 2023

View reviewed changes

gs-olive commented Feb 22, 2023

View reviewed changes

bowang007 approved these changes Feb 22, 2023

View reviewed changes

bowang007 force-pushed the fix_loop_fallback branch from 6f3ec49 to 5f43bf1 Compare February 22, 2023 19:49

gs-olive merged commit c0e82c8 into fix_loop_fallback Feb 22, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: Issue in non-Tensor Input Resolution #1617

fix: Issue in non-Tensor Input Resolution #1617

		cur_partitioned_block[i] =
		SegmentedBlock(cur_partitioned_block[i].get_id(), SegmentedBlock::kTensorRT, dependency_nodes);

fix: Issue in non-Tensor Input Resolution #1617

fix: Issue in non-Tensor Input Resolution #1617

Conversation

Description

Failure Case

Before Non-Tensor Input Resolution

After 1 Round of Non-Tensor Input Resolution

After 2 Rounds of Non-Tensor Input Resolution

Type of change

Checklist:

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment