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

Jump to bottom

[ONNX]Update Dropout Export #37641

Closed
wants to merge 19 commits into from
Closed

[ONNX]Update Dropout Export #37641

wants to merge 19 commits into from

Conversation

KsenijaS
Copy link
Contributor

@KsenijaS KsenijaS commented May 1, 2020 •
edited

Dropout operator in ONNX has additional input: training_mode.
Update Dropout export to match changes made in ONNX

@dr-ci
Copy link

dr-ci bot commented May 1, 2020 •
edited

💊 CI failures summary and remediations

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

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

ci.pytorch.org: 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.

See how this bot performed.

This comment has been revised 23 times.

@KsenijaS KsenijaS changed the title Update Dropout Export [ONNX]Update Dropout Export May 1, 2020
@KsenijaS
Copy link
Contributor Author

KsenijaS commented May 1, 2020

@lara-hdr @neginraoof @BowenBao please review

BowenBao
BowenBao approved these changes May 5, 2020
View reviewed changes
Copy link
Collaborator

@BowenBao BowenBao left a comment •
edited

Choose a reason for hiding this comment

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

LGTM

edit: failing onnx checker test in CI, looks like also need to update onnx version in CI.

lara-hdr
lara-hdr approved these changes May 5, 2020
View reviewed changes
Copy link
Contributor

@lara-hdr lara-hdr left a comment

Choose a reason for hiding this comment

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

@houseroad the CI is failing because ONNX submodule needs the be updated.

@KsenijaS
Copy link
Contributor Author

KsenijaS commented May 5, 2020

@houseroad is there an ETA for the update of the ONNX submodule?

@KsenijaS
Copy link
Contributor Author

KsenijaS commented May 14, 2020 •
edited

@houseroad I don't think build failure is related to the PR. LGTM

facebook-github-bot
facebook-github-bot reviewed May 17, 2020
View reviewed changes
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.

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

houseroad
houseroad approved these changes May 17, 2020
View reviewed changes
Copy link
Member

@houseroad houseroad left a comment

Choose a reason for hiding this comment

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

thanks

@facebook-github-bot
Copy link
Contributor

@houseroad merged this pull request in 5b12c29.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@facebook-github-bot facebook-github-bot facebook-github-bot left review comments

@lara-hdr lara-hdr lara-hdr approved these changes

@BowenBao BowenBao BowenBao approved these changes

@houseroad houseroad houseroad approved these changes

Assignees
No one assigned
Labels
Merged open source
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
7 participants
@KsenijaS @facebook-github-bot @BowenBao @lara-hdr @houseroad @pytorchbot @mruberry