-
Notifications
You must be signed in to change notification settings - Fork 3k
[core] Make the internal implementation of Core.compile_model(fs::path) the base implementation #33740
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
base: master
Are you sure you want to change the base?
[core] Make the internal implementation of Core.compile_model(fs::path) the base implementation #33740
Changes from all commits
e79b006
626f1c6
db81567
38df712
cf8a0f1
aed284d
627e64e
3e87302
31f50f5
04f100e
16cc4d3
34439ac
7a6f347
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -260,17 +260,12 @@ class OPENVINO_RUNTIME_API Core { | |
| */ | ||
| CompiledModel compile_model(const std::string& model_path, const AnyMap& properties = {}); | ||
|
|
||
| template <class Path, std::enable_if_t<std::is_same_v<Path, std::filesystem::path>>* = nullptr> | ||
| auto compile_model(const Path& model_path, const AnyMap& properties = {}) const { | ||
| if constexpr (std::is_same_v<typename Path::value_type, wchar_t>) | ||
| return compile_model(model_path.wstring(), properties); | ||
| else | ||
| return compile_model(model_path.string(), properties); | ||
| } | ||
| CompiledModel compile_model(const std::filesystem::path& model_path, const AnyMap& properties = {}); | ||
|
|
||
| #ifdef OPENVINO_ENABLE_UNICODE_PATH_SUPPORT | ||
| CompiledModel compile_model(const std::wstring& model_path, const AnyMap& properties = {}); | ||
| #endif | ||
| template <class Path, std::enable_if_t<std::is_constructible_v<std::string, Path>>* = nullptr> | ||
| CompiledModel compile_model(const Path& model_path, const AnyMap& properties = {}) { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. How is wstring version is handled?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. After I moved the implementation to
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. wstring -> path conversion may not work correctly in some cases, the test should be created to confirm it (see file_path.cpp) where there is dedicated constructor. It safer to keep wstring version and make call by using
almilosz marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| return compile_model(std::string(model_path), properties); | ||
| } | ||
| /// @} | ||
|
|
||
| /** | ||
|
|
@@ -281,34 +276,25 @@ class OPENVINO_RUNTIME_API Core { | |
| * especially for cases when caching is enabled and cached model is available | ||
| * | ||
| * @tparam Properties Should be the pack of `std::pair<std::string, ov::Any>` types | ||
| * @param model_path path to model with string or wstring | ||
| * @param model_path path to model | ||
| * @param properties Optional pack of pairs: (property name, property value) relevant only for this | ||
| * load operation | ||
| * | ||
| * @return A compiled model | ||
| * @{ | ||
| */ | ||
| template <typename... Properties> | ||
| util::EnableIfAllStringAny<CompiledModel, Properties...> compile_model(const std::string& model_path, | ||
| template <class Path, class... Properties, std::enable_if_t<std::is_constructible_v<std::string, Path>>* = nullptr> | ||
| util::EnableIfAllStringAny<CompiledModel, Properties...> compile_model(const Path& model_path, | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What about wstring version? |
||
| Properties&&... properties) { | ||
| return compile_model(model_path, AnyMap{std::forward<Properties>(properties)...}); | ||
| return compile_model(std::string(model_path), AnyMap{std::forward<Properties>(properties)...}); | ||
| } | ||
|
|
||
| template <class Path, class... Properties, std::enable_if_t<std::is_same_v<Path, std::filesystem::path>>* = nullptr> | ||
| auto compile_model(const Path& model_path, Properties&&... properties) { | ||
| if constexpr (std::is_same_v<typename Path::value_type, wchar_t>) | ||
| return compile_model(model_path.wstring(), std::forward<Properties>(properties)...); | ||
| else | ||
| return compile_model(model_path.string(), std::forward<Properties>(properties)...); | ||
| } | ||
|
|
||
| #ifdef OPENVINO_ENABLE_UNICODE_PATH_SUPPORT | ||
| template <typename... Properties> | ||
| util::EnableIfAllStringAny<CompiledModel, Properties...> compile_model(const std::wstring& model_path, | ||
| util::EnableIfAllStringAny<CompiledModel, Properties...> compile_model(const std::filesystem::path& model_path, | ||
| Properties&&... properties) { | ||
| return compile_model(model_path, AnyMap{std::forward<Properties>(properties)...}); | ||
| } | ||
| #endif | ||
|
|
||
| /// @} | ||
|
|
||
| /** | ||
|
|
@@ -329,19 +315,14 @@ class OPENVINO_RUNTIME_API Core { | |
| const std::string& device_name, | ||
| const AnyMap& properties = {}); | ||
|
|
||
| template <class Path, std::enable_if_t<std::is_same_v<Path, std::filesystem::path>>* = nullptr> | ||
| auto compile_model(const Path& model_path, const std::string& device_name, const AnyMap& properties = {}) { | ||
| if constexpr (std::is_same_v<typename Path::value_type, wchar_t>) | ||
| return compile_model(model_path.wstring(), device_name, properties); | ||
| else | ||
| return compile_model(model_path.string(), device_name, properties); | ||
| } | ||
|
|
||
| #ifdef OPENVINO_ENABLE_UNICODE_PATH_SUPPORT | ||
| CompiledModel compile_model(const std::wstring& model_path, | ||
| CompiledModel compile_model(const std::filesystem::path& model_path, | ||
| const std::string& device_name, | ||
| const AnyMap& properties = {}); | ||
| #endif | ||
| const AnyMap& config); | ||
|
|
||
| template <class Path, std::enable_if_t<std::is_constructible_v<std::string, Path>>* = nullptr> | ||
| CompiledModel compile_model(const Path& model_path, const std::string& device_name, const AnyMap& properties = {}) { | ||
| return compile_model(std::string(model_path), device_name, properties); | ||
almilosz marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| } | ||
| /// @} | ||
|
|
||
| /** | ||
|
|
@@ -359,29 +340,19 @@ class OPENVINO_RUNTIME_API Core { | |
| * @return A compiled model. | ||
| * @{ | ||
| */ | ||
| template <typename... Properties> | ||
| util::EnableIfAllStringAny<CompiledModel, Properties...> compile_model(const std::string& model_path, | ||
| template <class Path, class... Properties, std::enable_if_t<std::is_constructible_v<Path, std::string>>* = nullptr> | ||
| util::EnableIfAllStringAny<CompiledModel, Properties...> compile_model(const Path& model_path, | ||
| const std::string& device_name, | ||
| Properties&&... properties) { | ||
| return compile_model(model_path, device_name, AnyMap{std::forward<Properties>(properties)...}); | ||
| } | ||
|
|
||
| template <class Path, class... Properties, std::enable_if_t<std::is_same_v<Path, std::filesystem::path>>* = nullptr> | ||
| auto compile_model(const Path& model_path, const std::string& device_name, Properties&&... properties) { | ||
| if constexpr (std::is_same_v<typename Path::value_type, wchar_t>) | ||
| return compile_model(model_path.wstring(), device_name, std::forward<Properties>(properties)...); | ||
| else | ||
| return compile_model(model_path.string(), device_name, std::forward<Properties>(properties)...); | ||
| return compile_model(std::string(model_path), device_name, AnyMap{std::forward<Properties>(properties)...}); | ||
| } | ||
|
|
||
| #ifdef OPENVINO_ENABLE_UNICODE_PATH_SUPPORT | ||
| template <typename... Properties> | ||
| util::EnableIfAllStringAny<CompiledModel, Properties...> compile_model(const std::wstring& model_path, | ||
| util::EnableIfAllStringAny<CompiledModel, Properties...> compile_model(const std::filesystem::path& model_path, | ||
| const std::string& device_name, | ||
| Properties&&... properties) { | ||
| return compile_model(model_path, device_name, AnyMap{std::forward<Properties>(properties)...}); | ||
| } | ||
| #endif | ||
| /// @} | ||
|
|
||
| /** | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -9,6 +9,7 @@ | |
| #include "openvino/runtime/internal_properties.hpp" | ||
| #include "openvino/runtime/properties.hpp" | ||
| #include "openvino/util/common_util.hpp" | ||
| #include "openvino/util/file_util.hpp" | ||
|
|
||
| #define OV_PLUGIN_CALL_STATEMENT(...) \ | ||
| OPENVINO_ASSERT(m_ptr != nullptr, "OpenVINO Runtime Plugin was not initialized."); \ | ||
|
|
@@ -53,9 +54,9 @@ ov::SoPtr<ov::ICompiledModel> ov::Plugin::compile_model(const std::shared_ptr<co | |
| OV_PLUGIN_CALL_STATEMENT(return {m_ptr->compile_model(model, properties), m_so}); | ||
| } | ||
|
|
||
| ov::SoPtr<ov::ICompiledModel> ov::Plugin::compile_model(const std::string& model_path, | ||
| ov::SoPtr<ov::ICompiledModel> ov::Plugin::compile_model(const std::filesystem::path& model_path, | ||
| const ov::AnyMap& properties) const { | ||
| OV_PLUGIN_CALL_STATEMENT(return {m_ptr->compile_model(model_path, properties), m_so}); | ||
| OV_PLUGIN_CALL_STATEMENT(return {m_ptr->compile_model(ov::util::path_to_string(model_path), properties), m_so}); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I assume this path conversion will be remove when plugin interface will be updated?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes |
||
| } | ||
|
|
||
| ov::SoPtr<ov::ICompiledModel> ov::Plugin::compile_model(const std::shared_ptr<const ov::Model>& model, | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||
|---|---|---|---|---|
|
|
@@ -239,4 +239,38 @@ TEST_F(CoreBaseTest, compile_model_with_std_fs_path) { | |||
| } | ||||
| #endif | ||||
| } | ||||
|
|
||||
| TEST_F(CoreBaseTest, compile_model) { | ||||
| generate_test_model_files("model2"); | ||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What special unicode characters are created in path?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I wanted to reuse
|
||||
|
|
||||
| const auto model_path = model_file_name; | ||||
| const auto weight_path = weight_file_name; | ||||
|
Comment on lines
+246
to
+247
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why copy is required? |
||||
|
|
||||
| ov::Core core; | ||||
| { | ||||
| const auto model = core.compile_model(model_path); | ||||
| EXPECT_TRUE(model); | ||||
| } | ||||
| { | ||||
| const auto devices = core.get_available_devices(); | ||||
|
|
||||
| const auto model = core.compile_model(model_path, devices.at(0), ov::AnyMap{}); | ||||
| EXPECT_TRUE(model); | ||||
| } | ||||
| #ifdef OPENVINO_ENABLE_UNICODE_PATH_SUPPORT | ||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can be avoided similar like in |
||||
| for (std::size_t testIndex = 0; testIndex < ov::test::utils::test_unicode_postfix_vector.size(); testIndex++) { | ||||
| const auto model_path_w = model_files_name_w[testIndex]; | ||||
| { | ||||
| const auto model = core.compile_model(model_path_w); | ||||
| EXPECT_TRUE(model); | ||||
| } | ||||
| { | ||||
| const auto devices = core.get_available_devices(); | ||||
|
|
||||
| const auto model = core.compile_model(model_path_w, devices.at(0), ov::AnyMap{}); | ||||
| EXPECT_TRUE(model); | ||||
| } | ||||
| } | ||||
| #endif | ||||
| } | ||||
| } // namespace ov::test | ||||
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.
Instead remove it make if constexpr for string or wstring to call correct resolve overload use