Skip to content
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

[ONNX] Fix graph position to insert clone node for inplace op removal #50123

Merged
merged 3 commits into from Jan 31, 2021

Conversation

BowenBao
Copy link
Collaborator

@BowenBao BowenBao commented Jan 6, 2021

Previous insertBefore approach might end-up inserting clone node in inner sub-blocks, while then the node being used later at other outside call sites.

@facebook-github-bot facebook-github-bot added cla signed oncall: jit Add this issue/PR to JIT oncall triage queue labels Jan 6, 2021
@facebook-github-bot
Copy link
Contributor

facebook-github-bot commented Jan 6, 2021

💊 CI failures summary and remediations

As of commit 8aeda1f (more details on the Dr. CI page):


  • 1/1 failures introduced in this PR

🕵️ 1 new failure recognized by patterns

The following CI failures do not appear to be due to upstream breakages:

See CircleCI build pytorch_linux_backward_compatibility_check_test (1/1)

Step: "Run tests" (full log | diagnosis details | 🔁 rerun)

Jan 30 02:53:55 The PR is introducing backward incompatible changes to the operator library. Please contact PyTorch team to confirm whether this change is wanted or not.
Jan 30 02:53:55 processing existing schema:  __getstate__(__torch__.torch.classes.xnnpack.LinearOpContext _0) -> ((Tensor, Tensor?, Scalar?, Scalar?) _0)
Jan 30 02:53:55 processing existing schema:  __setstate__(__torch__.torch.classes.xnnpack.LinearOpContext _0, (Tensor, Tensor?, Scalar?, Scalar?) _1) -> (None _0)
Jan 30 02:53:55 processing existing schema:  __getstate__(__torch__.torch.classes.xnnpack.Conv2dOpContext _0) -> ((Tensor, Tensor?, int[], int[], int[], int, Scalar?, Scalar?) _0)
Jan 30 02:53:55 processing existing schema:  __setstate__(__torch__.torch.classes.xnnpack.Conv2dOpContext _0, (Tensor, Tensor?, int[], int[], int[], int, Scalar?, Scalar?) _1) -> (None _0)
Jan 30 02:53:55 processing existing schema:  __getstate__(__torch__.torch.classes.xnnpack.TransposeConv2dOpContext _0) -> ((Tensor, Tensor?, int[], int[], int[], int[], int, Scalar?, Scalar?) _0)
Jan 30 02:53:55 processing existing schema:  __setstate__(__torch__.torch.classes.xnnpack.TransposeConv2dOpContext _0, (Tensor, Tensor?, int[], int[], int[], int[], int, Scalar?, Scalar?) _1) -> (None _0)
Jan 30 02:53:55 processing existing schema:  __init__(__torch__.torch.classes._nnapi.Compilation _0) -> (None _0)
Jan 30 02:53:55 processing existing schema:  init(__torch__.torch.classes._nnapi.Compilation _0, Tensor _1, Tensor[] _2) -> (None _0)
Jan 30 02:53:55 processing existing schema:  run(__torch__.torch.classes._nnapi.Compilation _0, Tensor[] _1, Tensor[] _2) -> (None _0)
Jan 30 02:53:55 processing existing schema:  __init__(__torch__.torch.classes.dist_rpc.WorkerInfo _0, str _1, int _2) -> (None _0)
Jan 30 02:53:55 The PR is introducing backward incompatible changes to the operator library. Please contact PyTorch team to confirm whether this change is wanted or not. 
Jan 30 02:53:55 
Jan 30 02:53:55 Broken ops: [
Jan 30 02:53:55 	aten::fake_quantize_per_channel_affine_cachemask_backward(Tensor grad, Tensor mask) -> (Tensor)
Jan 30 02:53:55 	aten::fake_quantize_per_channel_affine_cachemask(Tensor self, Tensor scale, Tensor zero_point, int axis, int quant_min, int quant_max) -> (Tensor output, Tensor mask)
Jan 30 02:53:55 ]
Jan 30 02:53:55 + cleanup
Jan 30 02:53:55 + retcode=1
Jan 30 02:53:55 + set +x
Jan 30 02:53:55 =================== sccache compilation log ===================
Jan 30 02:53:55 =========== If your build fails, please take a look at the log above for possible reasons ===========

This comment was automatically generated by Dr. CI (expand for details).Follow this link to opt-out of these comments for your Pull Requests.

Please report bugs/suggestions to the (internal) Dr. CI Users group.

Copy link
Contributor

@neginraoof neginraoof left a comment

Choose a reason for hiding this comment

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

Thanks for fixing this.
Would you please show the graph for the test you added in comments?
Is this testing insertion into subblocks?

@BowenBao
Copy link
Collaborator Author

Thanks for fixing this.
Would you please show the graph for the test you added in comments?
Is this testing insertion into subblocks?

Showing before and after graph comparison.

# before
graph(%x.1 : Float(3, 16, strides=[16, 1], requires_grad=0, device=cpu),
      %fc.weight : Float(16, 16, strides=[16, 1], requires_grad=0, device=cpu),
      %fc.bias.1 : Float(16, strides=[1], requires_grad=0, device=cpu)):
  %3 : int = prim::Constant[value=2]() # /home/bowbao/repos/pytorch_test/torch/nn/functional.py:1663:22
  %4 : int = prim::Constant[value=1]()
  %5 : int = aten::dim(%x.1) # /home/bowbao/repos/pytorch_test/torch/nn/functional.py:1663:7
  %6 : bool = aten::eq(%5, %3) # /home/bowbao/repos/pytorch_test/torch/nn/functional.py:1663:7
  %out.1 : Tensor = prim::If(%6) # /home/bowbao/repos/pytorch_test/torch/nn/functional.py:1663:4
    block0():
      %8 : Tensor = aten::t(%fc.weight) # /home/bowbao/repos/pytorch_test/torch/nn/functional.py:1665:39
      %ret.2 : Tensor = aten::addmm(%fc.bias.1, %x.1, %8, %4, %4) # /home/bowbao/repos/pytorch_test/torch/nn/functional.py:1665:14
      -> (%ret.2)
    block1():
      %10 : Tensor = aten::t(%fc.weight) # /home/bowbao/repos/pytorch_test/torch/nn/functional.py:1667:30
      %22 : None = prim::Constant()
      ### <----- %fc.bias clone inserted here
      %fc.bias : Float(16, strides=[1], requires_grad=0, device=cpu) = aten::clone(%fc.bias.1, %22)
      %output.2 : Tensor = aten::matmul(%x.1, %10) # /home/bowbao/repos/pytorch_test/torch/nn/functional.py:1667:17
      %23 : Tensor = aten::add(%output.2, %fc.bias, %4) # /home/bowbao/repos/pytorch_test/torch/nn/functional.py:1669:12
      -> (%23)
  %13 : int = aten::dim(%out.1) # /home/bowbao/repos/pytorch_test/torch/nn/functional.py:1663:7
  %14 : bool = aten::eq(%13, %3) # /home/bowbao/repos/pytorch_test/torch/nn/functional.py:1663:7
  %out.3 : Tensor = prim::If(%14) # /home/bowbao/repos/pytorch_test/torch/nn/functional.py:1663:4
    block0():
      %16 : Tensor = aten::t(%fc.weight) # /home/bowbao/repos/pytorch_test/torch/nn/functional.py:1665:39
      ### <----- %fc.bias clone used in another block
      %ret.1 : Tensor = aten::addmm(%fc.bias, %out.1, %16, %4, %4) # /home/bowbao/repos/pytorch_test/torch/nn/functional.py:1665:14
      -> (%ret.1)
    block1():
      %18 : Tensor = aten::t(%fc.weight) # /home/bowbao/repos/pytorch_test/torch/nn/functional.py:1667:30
      %output.1 : Tensor = aten::matmul(%out.1, %18) # /home/bowbao/repos/pytorch_test/torch/nn/functional.py:1667:17
      %24 : Tensor = aten::add(%output.1, %fc.bias, %4) # /home/bowbao/repos/pytorch_test/torch/nn/functional.py:1669:12
      -> (%24)
  return (%out.3)
# after
graph(%x.1 : Float(3, 16, strides=[16, 1], requires_grad=0, device=cpu),
      %fc.weight : Float(16, 16, strides=[16, 1], requires_grad=0, device=cpu),
      %fc.bias.1 : Float(16, strides=[1], requires_grad=0, device=cpu)):
  %22 : None = prim::Constant()
  ### <----- %fc.bias clone inserted at the earliest after %fc.bias.1
  %fc.bias : Float(16, strides=[1], requires_grad=0, device=cpu) = aten::clone(%fc.bias.1, %22)
  %3 : int = prim::Constant[value=2]() # /home/bowbao/repos/pytorch_test/torch/nn/functional.py:1663:22
  %4 : int = prim::Constant[value=1]()
  %5 : int = aten::dim(%x.1) # /home/bowbao/repos/pytorch_test/torch/nn/functional.py:1663:7
  %6 : bool = aten::eq(%5, %3) # /home/bowbao/repos/pytorch_test/torch/nn/functional.py:1663:7
  %out.1 : Tensor = prim::If(%6) # /home/bowbao/repos/pytorch_test/torch/nn/functional.py:1663:4
    block0():
      %8 : Tensor = aten::t(%fc.weight) # /home/bowbao/repos/pytorch_test/torch/nn/functional.py:1665:39
      %ret.2 : Tensor = aten::addmm(%fc.bias.1, %x.1, %8, %4, %4) # /home/bowbao/repos/pytorch_test/torch/nn/functional.py:1665:14
      -> (%ret.2)
    block1():
      %10 : Tensor = aten::t(%fc.weight) # /home/bowbao/repos/pytorch_test/torch/nn/functional.py:1667:30
      %output.2 : Tensor = aten::matmul(%x.1, %10) # /home/bowbao/repos/pytorch_test/torch/nn/functional.py:1667:17
      %23 : Tensor = aten::add(%output.2, %fc.bias, %4) # /home/bowbao/repos/pytorch_test/torch/nn/functional.py:1669:12
      -> (%23)
  %13 : int = aten::dim(%out.1) # /home/bowbao/repos/pytorch_test/torch/nn/functional.py:1663:7
  %14 : bool = aten::eq(%13, %3) # /home/bowbao/repos/pytorch_test/torch/nn/functional.py:1663:7
  %out.3 : Tensor = prim::If(%14) # /home/bowbao/repos/pytorch_test/torch/nn/functional.py:1663:4
    block0():
      %16 : Tensor = aten::t(%fc.weight) # /home/bowbao/repos/pytorch_test/torch/nn/functional.py:1665:39
      ### <----- %fc.bias clone used here
      %ret.1 : Tensor = aten::addmm(%fc.bias, %out.1, %16, %4, %4) # /home/bowbao/repos/pytorch_test/torch/nn/functional.py:1665:14
      -> (%ret.1)
    block1():
      %18 : Tensor = aten::t(%fc.weight) # /home/bowbao/repos/pytorch_test/torch/nn/functional.py:1667:30
      %output.1 : Tensor = aten::matmul(%out.1, %18) # /home/bowbao/repos/pytorch_test/torch/nn/functional.py:1667:17
      %24 : Tensor = aten::add(%output.1, %fc.bias, %4) # /home/bowbao/repos/pytorch_test/torch/nn/functional.py:1669:12
      -> (%24)
  return (%out.3)

@BowenBao BowenBao force-pushed the onnx_remove_inplace_ops_fix branch 2 times, most recently from bb0c4ca to 6497181 Compare January 29, 2021 00:42
BowenBao added a commit that referenced this pull request Feb 2, 2021
… op removal (#50123)"

Previous insertBefore approach might end-up inserting clone node in inner sub-blocks, while then the node being used later at other outside call sites.

[ghstack-poisoned]
BowenBao added a commit that referenced this pull request Feb 3, 2021
… op removal (#50123)"

Previous insertBefore approach might end-up inserting clone node in inner sub-blocks, while then the node being used later at other outside call sites.

Differential Revision: [D26203124](https://our.internmc.facebook.com/intern/diff/D26203124)

[ghstack-poisoned]
facebook-github-bot pushed a commit that referenced this pull request Feb 4, 2021
…#50123) (#51520)

Summary:
Pull Request resolved: #51520

Previous insertBefore approach might end-up inserting clone node in inner sub-blocks, while then the node being used later at other outside call sites.

Test Plan: Imported from OSS

Reviewed By: pbelevich

Differential Revision: D26203124

Pulled By: SplitInfinity

fbshipit-source-id: 999511e901ad1087f360bb689fcdfc3743c78aa4
BowenBao added a commit to BowenBao/pytorch that referenced this pull request Feb 5, 2021
…pytorch#50123)

Previous insertBefore approach might end-up inserting clone node in inner sub-blocks, while then the node being used later at other outside call sites.

ghstack-source-id: e9179a4d525906e0197289e31072f895d289637e
Pull Request resolved: pytorch#51520
jiafatom pushed a commit to jiafatom/pytorch that referenced this pull request Feb 26, 2021
…pytorch#50123)

Previous insertBefore approach might end-up inserting clone node in inner sub-blocks, while then the node being used later at other outside call sites.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla signed oncall: jit Add this issue/PR to JIT oncall triage queue open source
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants