[ONNX] Improve error handling for adaptive_pool #43032

neginraoof · 2020-08-13T21:48:35Z

This would also improve error handling for interpolate with 'area' mode.

…oof/fixAdaptiveAvg

dr-ci · 2020-08-14T05:05:39Z

💊 CI failures summary and remediations

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

Commit c8fc4bf was recently pushed. Waiting for builds...

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 on the GitHub issue tracker or post in the (internal) Dr. CI Users group.

See how this bot performed.

This comment has been revised 52 times.

…oof/fixAdaptiveAvg

BowenBao

LG, thanks

facebook-github-bot

@bzinodev has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

facebook-github-bot

@bzinodev has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

…oof/fixAdaptiveAvg

neginraoof · 2020-09-01T19:09:19Z

@bzinodev
The failure in Jenkins: pr/pytorch-linux-bionic-rocm3.7-py3.6
is unrelated. I see this failure on other recent PR's as well.

Would you please let me know if this currently blocking merge? Or is there anything I can do to unblock this?

codecov · 2020-09-01T21:09:28Z

Codecov Report

Merging #43032 into master will decrease coverage by 0.00%.
The diff coverage is 0.00%.

@@            Coverage Diff             @@
##           master   #43032      +/-   ##
==========================================
- Coverage   68.05%   68.04%   -0.01%     
==========================================
  Files         396      396              
  Lines       51235    51238       +3     
==========================================
  Hits        34867    34867              
- Misses      16368    16371       +3

Impacted Files	Coverage Δ
torch/onnx/symbolic_opset9.py	`34.66% <0.00%> (-0.13%)`	⬇️
torch/onnx/utils.py	`72.06% <ø> (ø)`
torch/testing/_internal/expecttest.py	`78.57% <0.00%> (+1.02%)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 993628c...c8fc4bf. Read the comment docs.

facebook-github-bot

@bzinodev has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

facebook-github-bot

@bzinodev has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

…oof/fixAdaptiveAvg

facebook-github-bot

@bzinodev has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

neginraoof · 2020-09-15T20:31:04Z

@bzinodev
Can we please merge this PR?
Thanks!

neginraoof · 2020-09-18T17:41:27Z

@bzinodev
Can we please merge this PR?
Thanks!

…oof/fixAdaptiveAvg

neginraoof · 2020-09-21T23:00:06Z

@bzinodev Can we merge this please? Thanks!

facebook-github-bot

@bzinodev has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

facebook-github-bot

@bzinodev has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

…oof/fixAdaptiveAvg

facebook-github-bot

@bzinodev has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

neginraoof · 2020-09-29T18:44:13Z

cc @bzinodev
This PR is rebased and ready for merge. Thanks a lot.

bzinodev · 2020-10-02T21:00:58Z

torch/onnx/symbolic_opset9.py

        if mod != [0] * len(mod):
            if output_size == [1] * len(output_size):
                return g.op("GlobalMaxPool", input), None
-            return _unimplemented(name, 'output size that are not factor of input size')


Old behavior a warning is emitted. It is changed to a runtime error? there is one internal test that fails. Is this something we want?

Note other ops all uses _unimplemented

Before my update, this line in the symbolic conversion was throwing a warning instead of an error. Hence, the ATen node in the graph was not converted to ONNX, and was left as ATen.
By default, this behavior is not accepted. But exporter has a configuration mode that accepts combination of ONNX and ATen nodes. It's called ONNX_ATEN_FALLBACK.
It could be that the internal test case you see is failing is exporting with the ONNX_ATEN_FALLBACK mode.
I'll send a fix.

facebook-github-bot

@bzinodev has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

neginraoof · 2020-10-05T19:05:53Z

@bzinodev Could you please merge the last commit here? There was a typo that was fixed after I addressed your comment.

dzhulgakov · 2020-10-05T22:04:14Z

Hey, this PR unfortunately broke master (probably the missing typo you mentioned). I'm unlanding it, please resubmit the new PR with the fix, @bzinodev or myself can help you land it

facebook-github-bot · 2020-10-06T02:12:13Z

@bzinodev merged this pull request in 45ddeb5.

neginraoof · 2020-10-07T15:37:45Z

@bzinodev @dzhulgakov The new duplicate PR is: #45874
Could you please help merge this new one?
Thanks!

Summary: Duplicate of #43032 This update would also improve error handling for interpolate with 'area' mode. Pull Request resolved: #45874 Reviewed By: albanD Differential Revision: D24141266 Pulled By: bzinodev fbshipit-source-id: 7559f1d6af4f1ef3507c15a1aee76fe01fa433cd

Summary: Duplicate of pytorch#43032 This update would also improve error handling for interpolate with 'area' mode. Pull Request resolved: pytorch#45874 Reviewed By: albanD Differential Revision: D24141266 Pulled By: bzinodev fbshipit-source-id: 7559f1d6af4f1ef3507c15a1aee76fe01fa433cd

Summary: Duplicate of #43032 This update would also improve error handling for interpolate with 'area' mode. Pull Request resolved: #45874 Reviewed By: albanD Differential Revision: D24141266 Pulled By: bzinodev fbshipit-source-id: 7559f1d6af4f1ef3507c15a1aee76fe01fa433cd

Fix error handling for adaptive_pool

1b9e7c7

pytorchbot added the open source label Aug 13, 2020

Merge branch 'master' of https://github.com/pytorch/pytorch into nera…

b0f32ca

…oof/fixAdaptiveAvg

mrshenli added module: onnx Related to torch.onnx triaged This issue has been looked at a team member, and triaged and prioritized into an appropriate module labels Aug 18, 2020

mrshenli requested a review from houseroad August 18, 2020 17:31

neginraoof added 2 commits August 24, 2020 12:32

Update symbolic_opset9.py

d51be81

Merge branch 'master' of https://github.com/pytorch/pytorch into nera…

eb3e364

…oof/fixAdaptiveAvg

BowenBao approved these changes Aug 27, 2020

View reviewed changes

bzinodev approved these changes Aug 28, 2020

View reviewed changes

facebook-github-bot reviewed Aug 28, 2020

View reviewed changes

Update utils.py

2fc102d

facebook-github-bot reviewed Aug 31, 2020

View reviewed changes

Merge branch 'master' of https://github.com/pytorch/pytorch into nera…

d2ace5f

…oof/fixAdaptiveAvg

neginraoof added 3 commits September 1, 2020 19:04

Update symbolic_opset9.py

8aa83b4

Adding tests for exception handling

ff1a16f

Update test_pytorch_onnx_onnxruntime.py

1f68aa6

facebook-github-bot reviewed Sep 2, 2020

View reviewed changes

facebook-github-bot reviewed Sep 8, 2020

View reviewed changes

neginraoof added 3 commits September 8, 2020 17:57

Merge branch 'master' into neraoof/fixAdaptiveAvg

8a9cc37

Merge branch 'master' of https://github.com/pytorch/pytorch into nera…

c3a2b0e

…oof/fixAdaptiveAvg

Merge branch 'master' of https://github.com/pytorch/pytorch into nera…

12237b4

…oof/fixAdaptiveAvg

facebook-github-bot reviewed Sep 14, 2020

View reviewed changes

neginraoof changed the title ~~[ONNX] Imropve error handling for adaptive_pool~~ [ONNX] Improve error handling for adaptive_pool Sep 17, 2020

gchanan added this to the 1.7.0 milestone Sep 21, 2020

Merge branch 'master' of https://github.com/pytorch/pytorch into nera…

36d350b

…oof/fixAdaptiveAvg

facebook-github-bot reviewed Sep 22, 2020

View reviewed changes

Merge branch 'master' of https://github.com/pytorch/pytorch into nera…

fbe54e3

…oof/fixAdaptiveAvg

facebook-github-bot reviewed Sep 28, 2020

View reviewed changes

bzinodev reviewed Oct 2, 2020

View reviewed changes

Update symbolic_opset9.py

d0a9c70

facebook-github-bot reviewed Oct 2, 2020

View reviewed changes

Typo fix

c8fc4bf

facebook-github-bot closed this in 45ddeb5 Oct 5, 2020

neginraoof mentioned this pull request Oct 5, 2020

[ONNX] Improve error handling for adaptive_pool #45874

Closed

facebook-github-bot added the merged label Oct 6, 2020

neginraoof mentioned this pull request Oct 9, 2020

[Release/1.7] [ONNX] Improve error handling for adaptive_pool #46100

Merged

mruberry added the Merged label Oct 28, 2020

[ONNX] Improve error handling for adaptive_pool #43032

[ONNX] Improve error handling for adaptive_pool #43032

Uh oh!

Conversation

neginraoof commented Aug 13, 2020

Uh oh!

dr-ci bot commented Aug 14, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

💊 CI failures summary and remediations

Uh oh!

BowenBao left a comment

Choose a reason for hiding this comment

Uh oh!

facebook-github-bot left a comment

Choose a reason for hiding this comment

Uh oh!

facebook-github-bot left a comment

Choose a reason for hiding this comment

Uh oh!

neginraoof commented Sep 1, 2020

Uh oh!

codecov bot commented Sep 1, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

facebook-github-bot left a comment

Choose a reason for hiding this comment

Uh oh!

facebook-github-bot left a comment

Choose a reason for hiding this comment

Uh oh!

facebook-github-bot left a comment

Choose a reason for hiding this comment

Uh oh!

neginraoof commented Sep 15, 2020

Uh oh!

neginraoof commented Sep 18, 2020

Uh oh!

neginraoof commented Sep 21, 2020

Uh oh!

facebook-github-bot left a comment

Choose a reason for hiding this comment

Uh oh!

facebook-github-bot left a comment

Choose a reason for hiding this comment

Uh oh!

facebook-github-bot left a comment

Choose a reason for hiding this comment

Uh oh!

neginraoof commented Sep 29, 2020

Uh oh!

bzinodev Oct 2, 2020

Choose a reason for hiding this comment

Uh oh!

bzinodev Oct 2, 2020

Choose a reason for hiding this comment

Uh oh!

neginraoof Oct 2, 2020

Choose a reason for hiding this comment

Uh oh!

facebook-github-bot left a comment

Choose a reason for hiding this comment

Uh oh!

neginraoof commented Oct 5, 2020

Uh oh!

dzhulgakov commented Oct 5, 2020

Uh oh!

facebook-github-bot commented Oct 6, 2020

Uh oh!

neginraoof commented Oct 7, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

9 participants

dr-ci bot commented Aug 14, 2020 •

edited

Loading

codecov bot commented Sep 1, 2020 •

edited

Loading