Skip to content

Static protobuf for FEs#6588

Merged
ilyachur merged 28 commits intoopenvinotoolkit:masterfrom
ilya-lavrenov:StaticProtobufTests
Jul 16, 2021
Merged

Static protobuf for FEs#6588
ilyachur merged 28 commits intoopenvinotoolkit:masterfrom
ilya-lavrenov:StaticProtobufTests

Conversation

@ilya-lavrenov
Copy link
Copy Markdown
Contributor

Details:

  • item1
  • ...

Tickets:

  • ticket-id

@ilya-lavrenov ilya-lavrenov requested review from mbencer and nosovmik July 9, 2021 15:33
@ilya-lavrenov ilya-lavrenov force-pushed the StaticProtobufTests branch from 2290b05 to 7c6a4e2 Compare July 9, 2021 20:44
@ilya-lavrenov ilya-lavrenov marked this pull request as ready for review July 14, 2021 18:16
@ilya-lavrenov ilya-lavrenov requested a review from a team July 14, 2021 18:16
@ilya-lavrenov ilya-lavrenov requested review from a team as code owners July 14, 2021 18:16
@ilya-lavrenov ilya-lavrenov requested review from a team, ilyachur, slyalin and tomdol July 14, 2021 18:16
@ilya-lavrenov ilya-lavrenov changed the title Static protobuf tests Static protobuf for FEs Jul 14, 2021
@ilya-lavrenov ilya-lavrenov requested a review from tsocha July 14, 2021 18:41
#

set(TARGET_NAME "onnx_editor")
#set(TARGET_NAME "onnx_editor")
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Since this file is not needed, could you remove it?

{
public:
EdgeMapper() = default;
ONNX_IMPORTER_API EdgeMapper() = default;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Why exporting per method is used instead of standard 'class ONNX_IMPORTER_API EdgeMapper' (this way is used in other places of ngraph-core)?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I don't want to export ctor with protobufs entities

file(GLOB_RECURSE LIBRARY_HEADERS ${CMAKE_CURRENT_SOURCE_DIR}/src/*.hpp)
file(GLOB_RECURSE LIBRARY_PUBLIC_HEADERS ${CMAKE_CURRENT_SOURCE_DIR}/include/*.hpp)

file(GLOB_RECURSE EDITOR_LIBRARY_SRC ${CMAKE_CURRENT_SOURCE_DIR}/../onnx_editor/src/*.cpp)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

should folder structure be reorganized in future? like having only 'onnx' folder instead of onnx_import, onnx_editor, onnx_common

Copy link
Copy Markdown
Contributor Author

@ilya-lavrenov ilya-lavrenov Jul 15, 2021

Choose a reason for hiding this comment

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

yes, editor and importer will be somehow merged in a separate change

Currently, we want to focus on protobuf related questions

@ilyachur ilyachur merged commit 1621c66 into openvinotoolkit:master Jul 16, 2021
@ilya-lavrenov ilya-lavrenov deleted the StaticProtobufTests branch July 16, 2021 10:18
rnugmanx pushed a commit to rnugmanx/openvino that referenced this pull request Aug 26, 2021
* protobuf static + mixed editor and importer

* merged onnx common

* remove ngraph_test_util to onnx_importer and onnx

* styles applied

* Revert "remove ngraph_test_util to onnx_importer and onnx".

* test ngraph_test_util dependencies

* Fixed static protobuf

* Next

* Fixes

* Fixed cross-compilation

* Don't export / install onnx_proto

* Fixed Windows

* Fixed opencl headers

* Fix

* Added exclude-all for exe as well

* Fixed code style

* Try without LTO

* LTO off only for onnx importer

Co-authored-by: mbencer <mateusz.bencer@intel.com>
andrei-cv pushed a commit to andrei-cv/openvino that referenced this pull request Aug 30, 2021
* protobuf static + mixed editor and importer

* merged onnx common

* remove ngraph_test_util to onnx_importer and onnx

* styles applied

* Revert "remove ngraph_test_util to onnx_importer and onnx".

* test ngraph_test_util dependencies

* Fixed static protobuf

* Next

* Fixes

* Fixed cross-compilation

* Don't export / install onnx_proto

* Fixed Windows

* Fixed opencl headers

* Fix

* Added exclude-all for exe as well

* Fixed code style

* Try without LTO

* LTO off only for onnx importer

Co-authored-by: mbencer <mateusz.bencer@intel.com>
akuporos pushed a commit to akuporos/openvino that referenced this pull request Sep 29, 2021
* protobuf static + mixed editor and importer

* merged onnx common

* remove ngraph_test_util to onnx_importer and onnx

* styles applied

* Revert "remove ngraph_test_util to onnx_importer and onnx".

* test ngraph_test_util dependencies

* Fixed static protobuf

* Next

* Fixes

* Fixed cross-compilation

* Don't export / install onnx_proto

* Fixed Windows

* Fixed opencl headers

* Fix

* Added exclude-all for exe as well

* Fixed code style

* Try without LTO

* LTO off only for onnx importer

Co-authored-by: mbencer <mateusz.bencer@intel.com>
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.

7 participants