Rework model loading in FE manager, implement PDPD probing #6358

mvafin · 2021-06-24T20:32:51Z

Details:

Implement probing methods for FrontEndPDPD
Support reading PDPD model in ReadNetwork
Capabilities removed. It is expected that Frontend will do lazy initialization if called to do something
FrontEnds are now loaded relative to frontend_manager library, not "."

Tickets:

52258

ngraph/frontend/paddlepaddle/src/model.cpp

inference-engine/samples/benchmark_app/CMakeLists.txt

inference-engine/src/inference_engine/ie_network_reader.cpp

model-optimizer/mo/moc_frontend/pipeline.py

ngraph/frontend/frontend_manager/include/frontend_manager/frontend.hpp

cmake/templates/InferenceEngineConfig.cmake.in

inference-engine/src/inference_engine/ie_network_reader.cpp

inference-engine/tests/functional/inference_engine/CMakeLists.txt

inference-engine/tests/functional/inference_engine/pdpd_reader/read_pdpd_model_test.cpp

ilyachur · 2021-07-13T04:18:58Z

ngraph/frontend/frontend_manager/include/frontend_manager/frontend.hpp

+            virtual bool
+                supported_by_variants(const std::vector<std::shared_ptr<Variant>>& variants) const;


Looks like it is a service method, and I agree with @rkazants it shouldn't be a part of public API.

Or if it is a public method, why don't we have a documentation for it? I am a little bit confused, how should I implement this method in my own framework

ngraph/frontend/frontend_manager/include/frontend_manager/frontend.hpp

ilyachur · 2021-07-13T04:25:46Z

ngraph/frontend/paddlepaddle/include/paddlepaddle_frontend/model.hpp

+#else
+        typedef std::string path_type;
+#endif
+
        class PDPD_API InputModelPDPD : public InputModel


Can we make public API more clear and remove classes which won't be used by end user?

mvafin · 2021-07-13T12:02:57Z

public/developer API issue will be covered separately.

inference-engine/tests/functional/inference_engine/pdpd_reader/read_pdpd_model_test.cpp

ilyachur · 2021-07-14T07:37:47Z

ngraph/frontend/frontend_manager/include/frontend_manager/frontend.hpp

+            virtual bool
+                supported_by_variants(const std::vector<std::shared_ptr<Variant>>& variants) const;


@slyalin Do we have a plan to refactor API? Now it looks strange that we add service methods to public API

ngraph/frontend/frontend_manager/src/frontend_manager.cpp

ngraph/frontend/paddlepaddle/src/frontend.cpp

ilyachur · 2021-07-14T07:49:47Z

ngraph/frontend/paddlepaddle/src/pdpd_utils.hpp

+#endif
+
+            template <typename T>
+            bool endsWith(const std::basic_string<T>& str, const std::basic_string<T>& suffix)


It looks like a common util. should each frontend implement it?

You mean to create static util library?

Something like that. Moreover if I remember right we have the same methods in the Inference Engine

How can I access methods from IE? I thought we don't link IE to ngraph

In general it looks like it should be a common library. Could you add TODO to move such utility methods to common library and create a task?

* Remove supported_by_arguments from public api

ngraph/frontend/frontend_manager/include/frontend_manager/frontend_manager.hpp

mvafin · 2021-07-16T15:13:33Z

Ticket for further optimizing FrontendManager dependency is created: 60517.

ilya-lavrenov

Approved with the assumption that FEM will be split into 2 parts:

static library with base implementation for FEs
static / dynamic versions for FEM itself
static version of FEM is used in IE, dynamic is used by MO / end users

ngraph/frontend/frontend_manager/include/frontend_manager/frontend.hpp

ilyachur · 2021-07-19T04:47:55Z

ngraph/frontend/paddlepaddle/src/pdpd_utils.hpp

+#endif
+
+            template <typename T>
+            bool endsWith(const std::basic_string<T>& str, const std::basic_string<T>& suffix)


In general it looks like it should be a common library. Could you add TODO to move such utility methods to common library and create a task?

zhangYiIntel · 2021-07-22T07:03:46Z

ngraph/frontend/paddlepaddle/src/model.cpp

            FRONT_END_GENERAL_CHECK(m_fw_ptr->ParseFromIstream(&pb_stream),
                                    "Model can't be parsed");

            loadPlaces();
-            loadConsts(weights_stream ? "" : path, weights_stream.get());
+            loadConsts(weights_stream && weights_stream.is_open() ? std::basic_string<T>{} : path,


@nosovmik @mvafin This code only works with pdmodel & pdiparam files. However, there are legacy __model__ model files. If using __model__ The weights_stream is actually not valid.

Suggested change

loadConsts(weights_stream && weights_stream.is_open() ? std::basic_string<T>{} : path,

loadConsts(weights_stream && weights_stream.is_open() ? std::basic_string<T>{} : path,

weights_stream && weights_stream.is_open() ? &weights_stream : nullptr);

…oolkit#6358) * Rework model loading in FE manager, implement PDPD probing * Fix build * Fix build * Fix build * Fix unicode * Fix merge issues * Fix codestyle * Read frontends path from frontend_manager library location * Fix codestyle * Fix FE dependency * Fix dependencies * Fix codestyle * Check if model file exists * Revert adding model to lfs * Add test model * Fix cmake dependencies * Apply review feedback * Revert pugixml * make getFrontendLibraryPath not public API * Fix codestyle * Apply fix from Ilya Lavrenov * Add FE dependency in legacy tests * Remove not needed dependency * Better support Unicode * Fix build * Fix build * Fix build * Add dependency foe deprecated tests * Fix dependency * Fix typo * Revert adding FE dependency to IESharedTests * Remove relative paths from frontend unit tests * Apply review feedback * Fix typo * Return allow-undefined, since kmb dependecies fail to link * Fix merge conflict * Compare functions in reader tests * Simplify code to load from variants * Remove supported_by_arguments from public api * Fix codestyle * Fix build * Compare names in reader tests * Fix wchar in variant Co-authored-by: Ilya Churaev <[email protected]>

mvafin added 11 commits June 24, 2021 23:31

Rework model loading in FE manager, implement PDPD probing

236baa3

Fix build

6ea6cac

Fix build

e2aa495

Merge remote-tracking branch 'remotes/upstream/master'

d68f9b4

Fix build

5363588

Fix unicode

c067b50

Fix merge issues

de82907

Fix codestyle

e57d40f

Read frontends path from frontend_manager library location

38b0503

Fix codestyle

5561471

Fix FE dependency

79d3bcb

mvafin requested a review from nosovmik July 1, 2021 12:56

mvafin added 5 commits July 1, 2021 20:50

Fix dependencies

7880e77

Fix codestyle

9dd11e2

Check if model file exists

9fd49cd

Revert adding model to lfs

95f06e2

Add test model

835faae

mvafin marked this pull request as ready for review July 1, 2021 22:52

mvafin requested review from a team, ilyachur and slyalin July 1, 2021 22:52

nosovmik reviewed Jul 2, 2021

View reviewed changes

ngraph/frontend/paddlepaddle/src/model.cpp Outdated Show resolved Hide resolved

nosovmik reviewed Jul 2, 2021

View reviewed changes

inference-engine/samples/benchmark_app/CMakeLists.txt Outdated Show resolved Hide resolved

rkazants reviewed Jul 2, 2021

View reviewed changes

inference-engine/src/inference_engine/ie_network_reader.cpp Show resolved Hide resolved

rkazants reviewed Jul 2, 2021

View reviewed changes

model-optimizer/mo/moc_frontend/pipeline.py Show resolved Hide resolved

rkazants reviewed Jul 2, 2021

View reviewed changes

ngraph/frontend/frontend_manager/include/frontend_manager/frontend.hpp Outdated Show resolved Hide resolved

mvafin force-pushed the frontend/reader_master branch from 5f4b211 to 29d2367 Compare July 12, 2021 11:34

mvafin requested a review from nosovmik July 12, 2021 13:43

ilya-lavrenov added this to the 2022.1 milestone Jul 12, 2021

ilya-lavrenov added the category: inference OpenVINO Runtime library - Inference label Jul 12, 2021

ilyachur reviewed Jul 13, 2021

View reviewed changes

Compare functions in reader tests

ebf8b75

mvafin requested a review from ilyachur July 13, 2021 15:47

ilyachur reviewed Jul 14, 2021

View reviewed changes

mvafin added 3 commits July 15, 2021 15:56

Simplify code to load from variants

03e31f3

* Remove supported_by_arguments from public api

Fix codestyle

061ed5e

Fix build

50b200f

mvafin requested a review from ilyachur July 15, 2021 13:52

mvafin assigned ilyachur Jul 15, 2021

mvafin requested a review from tomdol July 16, 2021 07:34

Compare names in reader tests

3012de8

slyalin reviewed Jul 16, 2021

View reviewed changes

ngraph/frontend/frontend_manager/include/frontend_manager/frontend_manager.hpp Show resolved Hide resolved

Fix wchar in variant

8465af5

slyalin approved these changes Jul 16, 2021

View reviewed changes

ilya-lavrenov approved these changes Jul 17, 2021

View reviewed changes

ilyachur approved these changes Jul 19, 2021

View reviewed changes

Merge branch 'master' into frontend/reader_master

b2b11e7

nosovmik approved these changes Jul 19, 2021

View reviewed changes

slyalin merged commit 960ba48 into openvinotoolkit:master Jul 19, 2021

nosovmik mentioned this pull request Jul 19, 2021

[ONNX] Add ONNX specific FrontEnd classes #6615

Merged

zhangYiIntel reviewed Jul 22, 2021

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Rework model loading in FE manager, implement PDPD probing #6358

Rework model loading in FE manager, implement PDPD probing #6358

mvafin commented Jun 24, 2021 •

edited

Loading

ilyachur Jul 13, 2021

ilyachur Jul 13, 2021

mvafin commented Jul 13, 2021

ilyachur Jul 14, 2021

ilyachur Jul 14, 2021

mvafin Jul 14, 2021

ilyachur Jul 15, 2021

mvafin Jul 15, 2021

ilyachur Jul 19, 2021

mvafin commented Jul 16, 2021

ilya-lavrenov left a comment

ilyachur Jul 19, 2021

zhangYiIntel Jul 22, 2021

		virtual bool
		supported_by_variants(const std::vector<std::shared_ptr<Variant>>& variants) const;

	loadConsts(weights_stream && weights_stream.is_open() ? std::basic_string<T>{} : path,
	loadConsts(weights_stream && weights_stream.is_open() ? std::basic_string<T>{} : path,
	weights_stream && weights_stream.is_open() ? &weights_stream : nullptr);

Rework model loading in FE manager, implement PDPD probing #6358

Rework model loading in FE manager, implement PDPD probing #6358

Conversation

mvafin commented Jun 24, 2021 • edited Loading

Details:

Tickets:

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mvafin commented Jul 13, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mvafin commented Jul 16, 2021

ilya-lavrenov left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mvafin commented Jun 24, 2021 •

edited

Loading