-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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] Add ONNX specific FrontEnd classes #6615
Merged
postrational
merged 15 commits into
openvinotoolkit:master
from
mateusztabaka:onnx_frontend
Jul 22, 2021
Merged
Changes from 6 commits
Commits
Show all changes
15 commits
Select commit
Hold shift + click to select a range
b8513c0
[ONNX] Add ONNX specific FrontEnd classes
mateusztabaka 01b5179
rename NGRAPH_ONNX_EDITOR_ENABLE to NGRAPH_ONNX_FRONTEND_ENABLE
mateusztabaka 8d79baf
fix error message
mateusztabaka 15df53a
Merge branch 'master' into onnx_frontend
mateusztabaka 0b9c298
apply code style
mateusztabaka c7c41aa
disable ONNX frontend by default
mateusztabaka 08fdfe8
Merge branch 'master' into onnx_frontend
mateusztabaka f74740f
apply code style
mateusztabaka 6b1add7
apply review comments
mateusztabaka fe3ddab
add ONNX_FRONTEND_API macro
mateusztabaka f3f7e38
Merge branch 'master' into onnx_frontend
mateusztabaka 1b8c82c
apply code style
mateusztabaka 92552d8
fix flake style
mateusztabaka 7fd0b18
Merge branch 'master' into onnx_frontend
mateusztabaka 4907023
Merge branch 'master' into onnx_frontend
mateusztabaka File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can frontend be enabled by default everywhere when NGRAPH_ONNX_IMPORT_ENABLE is ON? This is to make sure that 'frontend' can be correctly built on all supported platforms
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agree, don't see a reason how have multiple options about the similar stuff
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In this case, maybe set name for onnx frontend as something temporary, like "onnx_experimental"? So that it will not conflict with legacy 'onnx' name in MO. I just want to be sure that there are no build issues of onnx frontend on supported platforms
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The feature is currently under development. We have separate PRs for testing against CI with the feature turned ON and we'll merge them after we complete the feature. For now the feature will be turned OFF be defautlt.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How can we merge changes when we don't know if they are even compilable?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We manually enable
NGRAPH_ONNX_FRONTEND_ENABLE=ON
in a number of CI environments. We are just not ready to turn it on by default, especially in the context of Model Optimizer.