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] Handle dynamic input axes for prim_ConstantChunk #48176

Closed
wants to merge 21 commits into from

Conversation

jiafatom
Copy link
Contributor

@jiafatom jiafatom commented Nov 18, 2020

When converting a model that uses torch.chunk, it does not work when we have a dynamic input axes, because Split split attr is static for opset 11. Therefore, we convert it using Slice (support opset 11+). This PR also handles the cases that the input axes cannot be divided by the number of outputs. Pytorch works a way that fit the first (n-1) outputs for the same dim, and remaining for the last one. Added UT for it.

The existing code on sequence split cannot be leveraged here, because start, end of Slice are static there, but dynamic here.

@jiafatom jiafatom changed the title Handle dynamic input axes for prim_ConstantChunk [ONNX] Handle dynamic input axes for prim_ConstantChunk Nov 18, 2020
@dr-ci
Copy link

dr-ci bot commented Nov 18, 2020

💊 CI failures summary and remediations

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


  • 2/2 failures possibly* introduced in this PR
    • 2/2 non-CircleCI failure(s)

Extra GitHub checks: 1 failed


codecov.io: 1 failed


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 20 times.

@zhangguanheng66 zhangguanheng66 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 Nov 18, 2020
@codecov
Copy link

codecov bot commented Nov 30, 2020

Codecov Report

Merging #48176 (1929925) into master (0e5682d) will decrease coverage by 0.00%.
The diff coverage is 44.82%.

@@            Coverage Diff             @@
##           master   #48176      +/-   ##
==========================================
- Coverage   80.92%   80.92%   -0.01%     
==========================================
  Files        1855     1855              
  Lines      200195   200224      +29     
==========================================
+ Hits       162009   162025      +16     
- Misses      38186    38199      +13     

Copy link
Collaborator

@BowenBao BowenBao left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@facebook-github-bot facebook-github-bot left a comment

Choose a reason for hiding this comment

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

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

@facebook-github-bot
Copy link
Contributor

@bzinodev merged this pull request in f065087.

@facebook-github-bot
Copy link
Contributor

@bzinodev merged this pull request in f065087.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla signed Merged module: onnx Related to torch.onnx open source triaged This issue has been looked at a team member, and triaged and prioritized into an appropriate module
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants