Skip to content

✨[Feature] Do not remove nodes which are in-place operations (classified in TRT segments) in fallback #1121

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

Closed
peri044 opened this issue Jun 15, 2022 · 7 comments
Assignees
Labels
bug Something isn't working component: partitioning feature request New feature or request No Activity

Comments

@peri044
Copy link
Collaborator

peri044 commented Jun 15, 2022

Is your feature request related to a problem? Please describe.
Currently for eg: if aten::append is in prim::If block, it gets removed because it is classified as TRT segment. No output is registered for it because aten::append is an inplace op. Make modifications to registerSegmentsOutputs

Segmented Graph: graph(%self_1 : __torch__.ModifyingOpError_trt,
      %x.1 : Tensor,
      %y.1 : Tensor):
  %mod_list.1 : Tensor[] = prim::ListConstruct(%x.1)
  %__torch___ModifyingOpError_trt_engine_0x557361771710 : __torch__.torch.classes.tensorrt.Engine = prim::GetAttr[name="__torch___ModifyingOpError_trt_engine_0x557361771710"](%self_1)
  %5 : Tensor[] = prim::ListConstruct(%x.1, %y.1)
  %6 : Tensor[] = tensorrt::execute_engine(%5, %__torch___ModifyingOpError_trt_engine_0x557361771710)
  %7 : Tensor = prim::ListUnpack(%6)
  %8 : bool = aten::Bool(%7) # mod.py:12:11
   = prim::If(%8)
    block0():
      -> ()
    block1():
      -> ()
  %9 : int = prim::Constant[value=0]()
  %10 : Tensor = aten::cat(%mod_list.1, %9) # mod.py:14:12
  return (%10)

Describe the solution you'd like

Describe alternatives you've considered

Additional context

@peri044 peri044 added the feature request New feature or request label Jun 15, 2022
@peri044
Copy link
Collaborator Author

peri044 commented Jun 15, 2022

Also add this as a test case for this PR #1067

class ModifyingOpError(nn.Module):
    def __init__(self):
        super(ModifyingOpError, self).__init__()
    def forward(self, x, y):
        mod_list = [x]
        if x.sum() > y.sum():
            mod_list.append(y)
        z = torch.cat(mod_list)
        return z

@ncomly-nvidia
Copy link
Contributor

Resolved by #1140. Closing

@bowang007
Copy link
Collaborator

@ncomly-nvidia I don't this this one should be closed.
It refers to whether we should delete these several lines

segmented_blocks.erase(

@github-actions
Copy link

This issue has not seen activity for 90 days, Remove stale label or comment or this will be closed in 10 days

@ncomly-nvidia ncomly-nvidia added the bug Something isn't working label Nov 7, 2022
@narendasan
Copy link
Collaborator

@gs-olive / @bowang007 How does this relate to index_put_?

@gs-olive
Copy link
Collaborator

gs-olive commented Dec 15, 2022

@gs-olive / @bowang007 How does this relate to index_put_?

I don't think this specific case is directly related to index_put_ other than that both append and index_put_ are in-place operations. In this case, the issue seems to be that no-return if...else code blocks are removed, while they could still be making meaningful changes to existing tensors (in-place). Regarding index_put_, if it were located within an if...else block without a return value in TorchScript like the above, it would also be optimized away, though this holds true for any in-place operation.

I think in both the case of append and index_put_, functionalizing the graph as much as possible is very important, especially considering proposed new features including allowing Int64 inputs (RFC #1553), or debugging in-place issues related to assigning values to TRT Constant-labeled tensors, as in Issue #1453

@github-actions
Copy link

This issue has not seen activity for 90 days, Remove stale label or comment or this will be closed in 10 days

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working component: partitioning feature request New feature or request No Activity
Projects
None yet
Development

No branches or pull requests

5 participants