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 sequence output shape and type inference #46542

Closed
wants to merge 233 commits into from

Conversation

neginraoof
Copy link
Contributor

@neginraoof neginraoof commented Oct 19, 2020

Handle sequence output shape and type inference.

This PR fixes value type of sequence outputs. Prior to this, all model sequence type outputs were unfolded for ONNX models.
This PR also enable shape inference for sequence outputs to represent the dynamic shape of these values.

…oof/functionalization

# Conflicts:
#	aten/src/ATen/core/ivalue.h
#	aten/src/ATen/core/ivalue_inl.h
#	aten/src/ATen/native/TestOps.cpp
#	aten/src/ATen/native/native_functions.yaml
#	test/test_native_functions.py
#	tools/autograd/gen_autograd_functions.py
#	tools/jit/gen_unboxing_wrappers.py
#	torch/csrc/utils/python_arg_parser.h
@neginraoof
Copy link
Contributor Author

@bzinodev
Would you please help us merge this PR? This one is the last PR we would like you to merge to master, and it is a high priority PR.

Thanks a lot.

Copy link
Contributor

@bzinodev bzinodev left a comment

Choose a reason for hiding this comment

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

This is a change to ONNX export API as know it can return lists.

We need more description of this change and discuss if we are break backward compatibility

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.

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.

@neginraoof
Copy link
Contributor Author

@bzinodev
Is it okay to merge this PR now?

…orch into neraoof/opset13

# Conflicts:
#	test/onnx/test_pytorch_onnx_onnxruntime.py
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 adc65e7.

@neginraoof
Copy link
Contributor Author

PR to fix broken mypy tests: #50571

@nairbv
Copy link
Collaborator

nairbv commented Jan 15, 2021

Hi @neginraoof, there's an "unland" for this diff that's already in progress (we actively revert changes when tests fail). Can you re-submit the original diff including the fix from 50571 (once the reverting change finishes landing)?

@facebook-github-bot
Copy link
Contributor

This pull request has been reverted by a9db2f8.

facebook-github-bot pushed a commit that referenced this pull request Jan 21, 2021
Summary:
Duplicate of #46542

Pull Request resolved: #50599

Reviewed By: SplitInfinity

Differential Revision: D25928897

Pulled By: bzinodev

fbshipit-source-id: a898cef7b2d15a287aedd9798ce1423cebf378d4
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla signed Merged oncall: jit Add this issue/PR to JIT oncall triage queue open source Reverted 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

7 participants