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

[Static Runtime] Fix aten::clone out variant (#78297) #78322

Closed

Conversation

akshayParashar1995
Copy link
Contributor

Summary:
Pull Request resolved: #78297

Clone followed by expand/expand_as due to memoryOverlap check on copy_ native method. Refer to T118519310 for more details.

Crashing test case:
a = tensor(3,1) // strides = (1,1)
B = tensor(3,2) // strides = (2,1)
Temp = a.expand_as(b). // creates temp with shape as (3,2) and strides as (1,0)
temp.clone() // crashe on copy_ due to memoryOverlap

Fix: Disable the out variant for the expanded tensor.

  • Calls native clone instead of out variant for clone dealing with expanded tensors
  • Added test case for both clone variants (out and native clones)
  • Increased the tensor size for memory planner test case to trigger dynamic allocation

Test Plan:
buck test caffe2/benchmarks/static_runtime/fb:test_fb_operators

buck test caffe2/benchmarks/static_runtime:static_runtime_cpptest

Differential Revision: D36672180

@facebook-github-bot
Copy link
Contributor

facebook-github-bot commented May 25, 2022

🔗 Helpful links

❌ 1 New Failures, 1 Base Failures

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

Expand to see more
  • 1/2 failures introduced in this PR
  • 1/2 broken upstream at merge base 308d813 on Jun 02 from 6:40am to 1:15pm

🕵️ 1 new failure recognized by patterns

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

See GitHub Actions build pull / linux-xenial-py3.7-clang7-onnx / test (default, 1, 2, linux.2xlarge) (1/1)

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

2022-06-02T17:35:58.9351241Z AssertionError: St...'onnx::SequenceConstruct' != 'prim::ListConstruct'
2022-06-02T17:35:58.9329666Z 
2022-06-02T17:35:58.9335384Z ――――――――――――――――― TestUtilityFuns_opset9.test_prim_fallthrough ―――――――――――――――――
2022-06-02T17:35:58.9340339Z [gw1] linux -- Python 3.7.13 /opt/conda/bin/python
2022-06-02T17:35:58.9346643Z Traceback (most recent call last):
2022-06-02T17:35:58.9347221Z   File "/var/lib/jenkins/workspace/test/onnx/test_utility_funs.py", line 1153, in test_prim_fallthrough
2022-06-02T17:35:58.9347870Z     self.assertEqual(next(iter).kind(), "prim::ListConstruct")
2022-06-02T17:35:58.9348669Z   File "/opt/conda/lib/python3.7/site-packages/torch/testing/_internal/common_utils.py", line 2286, in assertEqual
2022-06-02T17:35:58.9349316Z     msg=(lambda generated_msg: f"{generated_msg} : {msg}") if isinstance(msg, str) and self.longMessage else msg,
2022-06-02T17:35:58.9350083Z   File "/opt/conda/lib/python3.7/site-packages/torch/testing/_comparison.py", line 1095, in assert_equal
2022-06-02T17:35:58.9350561Z     raise error_metas[0].to_error(msg)
2022-06-02T17:35:58.9351241Z AssertionError: String comparison failed: 'onnx::SequenceConstruct' != 'prim::ListConstruct'
2022-06-02T17:35:58.9351802Z - onnx::SequenceConstruct
2022-06-02T17:35:58.9352131Z + prim::ListConstruct
2022-06-02T17:35:58.9352306Z 
2022-06-02T17:35:58.9361812Z 
2022-06-02T17:35:58.9361823Z 
2022-06-02T17:35:58.9365160Z 
2022-06-02T17:35:58.9365172Z 
2022-06-02T17:35:58.9372633Z [gw1] �[31mFAILED�[0m test/onnx/test_utility_funs.py �[1A
2022-06-02T17:35:58.9539024Z  �[36mtest/onnx/test_utility_funs.py�[0m::TestUtilityFuns_opset9.test_prim_fallthrough�[0m �[31m⨯�[0m�[31m96% �[0m�[40m�[32m█�[0m�[40m�[32m█�[0m�[40m�[32m█�[0m�[40m�[32m█�[0m�[40m�[31m█�[0m�[40m�[32m█�[0m�[40m�[32m█�[0m�[40m�[32m█�[0m�[40m�[32m█�[0m�[40m�[31m▋�[0m�[1B
2022-06-02T17:35:58.9539614Z 

🚧 1 fixed upstream failure:

These were probably caused by upstream breakages that were already fixed.

Please rebase on the viable/strict branch (expand for instructions)

If your commit is older than viable/strict, run these commands:

git fetch https://github.com/pytorch/pytorch viable/strict
git rebase FETCH_HEAD

This comment was automatically generated by Dr. CI (expand for details).

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

Click here to manually regenerate this comment.

@facebook-github-bot facebook-github-bot added cla signed oncall: jit Add this issue/PR to JIT oncall triage queue fb-exported labels May 25, 2022
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D36672180

1 similar comment
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D36672180

@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D36672180

@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D36672180

@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D36672180

@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D36672180

@mikeiovine mikeiovine self-requested a review June 1, 2022 00:57
Summary:
Pull Request resolved: pytorch#78322

Disable the out variant for tensors with stride 0 and non-preserve memoryFormats.
- Calls native clone instead of out variant for clone dealing with expanded tensors
- Added test case for both clone variants (out and native clones).
- Increased the tensor size for memory planner test case to trigger dynamic allocation

Test Plan:
buck test caffe2/benchmarks/static_runtime/fb:test_fb_operators

buck test caffe2/benchmarks/static_runtime:static_runtime_cpptest

Reviewed By: mikeiovine, tenpercent

Differential Revision: D36672180

fbshipit-source-id: 48eb9533867d91ab3dc1b03be5750885818c33e5
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D36672180

@facebook-github-bot
Copy link
Contributor

@pytorchbot merge

(Initiating merge automatically since Phabricator Diff has merged)

facebook-github-bot pushed a commit that referenced this pull request Jun 2, 2022
Summary:
Pull Request resolved: #78322

Disable the out variant for tensors with stride 0 and non-preserve memoryFormats.
- Calls native clone instead of out variant for clone dealing with expanded tensors
- Added test case for both clone variants (out and native clones).
- Increased the tensor size for memory planner test case to trigger dynamic allocation

Test Plan:
buck test caffe2/benchmarks/static_runtime/fb:test_fb_operators

buck test caffe2/benchmarks/static_runtime:static_runtime_cpptest

Reviewed By: mikeiovine, tenpercent

Differential Revision: D36672180

fbshipit-source-id: 64263ad3d3950d95a39e2213345aa9f20746779f
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla signed fb-exported Merged oncall: jit Add this issue/PR to JIT oncall triage queue
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants