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

Update onnx submodule to 1.7.0 release candidate #3405

Merged
merged 8 commits into from Apr 4, 2020
Merged

Update onnx submodule to 1.7.0 release candidate #3405

merged 8 commits into from Apr 4, 2020

Conversation

snnn
Copy link
Member

@snnn snnn commented Apr 2, 2020

Description:

Update onnx submodule to 1.7.0 release candidate

Motivation and Context

  • Why is this change required? What problem does it solve?

This is a chicken-egg problem. If we don't do this, we don't know if ONNX is ready to be released.

  • If it fixes an open issue, please link to the issue here.

@snnn snnn requested a review from a team as a code owner April 2, 2020 05:03
@codemzs
Copy link
Member

codemzs commented Apr 2, 2020

Hi @snnn I'm already this doing this in my PR that has been out since yesterday. I will appreciate if you please close this PR to avoid conflicts.

@snnn
Copy link
Member Author

snnn commented Apr 2, 2020

Hi @snnn I'm already this doing this in my PR that has been out since yesterday. I will appreciate if you please close this PR to avoid conflicts.

ONNX 1.7 release has been delayed a few weeks. We really want to get it done. I will appreciate if you can help ONNX community validate the release candidate. It would be good for all of us. When we start to taking the release to onnxruntime, if any major bug was found, we'll stop there and won't take it until the bug get fixed. In that case, the merging of training branch to master will also be delayed. I don't want to see it happan, that's why I'm spending time here. There is no conflict between your effort and mine, we're all towards to the same goal.

I'm not hurried to merge this PR, because there are many failures needed be fixed, which takes time. If you want to help, I really appreciate it. Please submit your changes to the master branch, because most of the devs work on master, and many of them are waiting this change to get started working on opset 12 kernels. This is urgent.

@codemzs
Copy link
Member

codemzs commented Apr 2, 2020

@snnn I understand this is urgent and you have to also understand I have been finding and fixing bugs w.r.t ONNX 1.7 release, please see onnx/onnx#2680 onnx/onnx#2667 and then #3392 that implements the kernel and re-enables disabled tests that were previously failing.

@snnn
Copy link
Member Author

snnn commented Apr 2, 2020

@snnn I understand this is urgent and you have to also understand I have been finding and fixing bugs w.r.t ONNX 1.7 release,

Yes I know. Really thanks.

Now the ONNX community want to know if the ONNX rel-1.7.0 branch is ready for release. So the release manager came to me and asked me helping me do a verification. He is waiting a response from me and I did find something in the last round of testing. If you also find something wrong in ONNX or If you're doing something that he should wait, please let us know.

@yuslepukhin
Copy link
Member

yuslepukhin commented Apr 3, 2020

Cchasun@microsoft.com I need these changes, please, go ahead. This is blocking OpSet12 implementation


In reply to: 607661043 [](ancestors = 607661043)

yuslepukhin
yuslepukhin previously approved these changes Apr 3, 2020
Copy link
Member

@yuslepukhin yuslepukhin left a comment

Choose a reason for hiding this comment

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

:shipit:

@snnn snnn requested a review from yuslepukhin April 4, 2020 22:57
@souptc souptc self-requested a review April 4, 2020 23:16
@snnn snnn merged commit 33006f4 into master Apr 4, 2020
@snnn snnn deleted the snnn/onnx2 branch April 4, 2020 23:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants