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.

Sign up for GitHub

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

Check torch onnx export validity #78

Merged
merged 5 commits into from
Jan 9, 2024
Merged

Check torch onnx export validity #78

merged 5 commits into from
Jan 9, 2024

Conversation

ramkrishna2910
Copy link
Collaborator

This PR adds the ability to verify if the torch onnx exporter broke the model during export.
We dont throw an error upon failure of validity check, rather capture this info in the turnkey_stats.yaml file as shown below.

image

@ramkrishna2910 ramkrishna2910 added the bug Something isn't working label Jan 9, 2024
@ramkrishna2910 ramkrishna2910 self-assigned this Jan 9, 2024
jeremyfowers
jeremyfowers requested changes Jan 9, 2024
View reviewed changes
Copy link
Collaborator

@jeremyfowers jeremyfowers left a comment

Choose a reason for hiding this comment

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

Looks good but could you please provide some clarifications before merging?

jeremyfowers
jeremyfowers approved these changes Jan 9, 2024
View reviewed changes
Copy link
Collaborator

@jeremyfowers jeremyfowers left a comment

Choose a reason for hiding this comment

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

Nice change, and thanks for the good discussion! I'm a little concerned about the export getting 2x longer, let's keep an eye on that.

@ramkrishna2910 ramkrishna2910 enabled auto-merge (squash) January 9, 2024 22:07
@ramkrishna2910 ramkrishna2910 merged commit 8204801 into main Jan 9, 2024
10 checks passed
@ramkrishna2910 ramkrishna2910 deleted the ramkrishna2910/check-onnx-export branch January 9, 2024 22:12
jeremyfowers added a commit that referenced this pull request Jan 10, 2024
@jeremyfowers
Add release note for #78
cfd8ce0
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jeremyfowers jeremyfowers jeremyfowers approved these changes

@danielholanda danielholanda Awaiting requested review from danielholanda

Assignees

@ramkrishna2910 ramkrishna2910

Labels
bug Something isn't working
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@ramkrishna2910 @jeremyfowers