Code review

openvinotoolkit · Aug 20, 2024 · f2ab3f3 · f2ab3f3
1 parent e46879c
commit f2ab3f3
Show file tree

Hide file tree

Showing 6 changed files with 48 additions and 62 deletions.
diff --git a/samples/cpp/continuous_batching_benchmark/continuous_batching_benchmark.cpp b/samples/cpp/continuous_batching_benchmark/continuous_batching_benchmark.cpp
@@ -499,7 +499,7 @@ int main(int argc, char* argv[]) try {
     std::cout << "Loading models, creating pipelines, preparing environment..." << std::endl;
     ov::genai::ContinuousBatchingPipeline pipe(models_path, scheduler_config, device, device_config_map);
 
-    pipe.print_model_configuration();
+    std::cout << "Model configuration: " << std::endl << pipe.get_model_configuration_string();
 
     std::cout << "Setup finished, launching LLM executor, traffic simulation and statistics reporter threads" << std::endl;
 

diff --git a/src/cpp/include/openvino/genai/continuous_batching_pipeline.hpp b/src/cpp/include/openvino/genai/continuous_batching_pipeline.hpp
@@ -61,9 +61,7 @@ class OPENVINO_GENAI_EXPORTS ContinuousBatchingPipeline {
     GenerationHandle add_request(uint64_t request_id, const ov::Tensor& input_ids, const ov::genai::GenerationConfig& sampling_params);
     GenerationHandle add_request(uint64_t request_id, const std::string& prompt, const ov::genai::GenerationConfig& sampling_params);
 
-    std::vector<std::string> get_model_configuration();
-
-    void print_model_configuration();
+    std::string get_model_configuration_string();
 
     void step();
 

diff --git a/src/cpp/src/continuous_batching_pipeline.cpp b/src/cpp/src/continuous_batching_pipeline.cpp
@@ -15,6 +15,7 @@
 #include "text_callback_streamer.hpp"
 #include "timer.hpp"
 #include "debug_utils.hpp"
+#include "utils.hpp"
 
 using namespace ov::genai;
 
@@ -99,7 +100,7 @@ class ContinuousBatchingPipeline::Impl {
         apply_paged_attention_transformations(model, device_config);
 
         auto compiled_model = core.compile_model(model, device_config.get_device(), plugin_config);
-        read_properties([compiled_model](const std::string& key) {
+        ov::genai::utils::read_properties([compiled_model](const std::string& key) {
             return compiled_model.get_property(key); },
             m_model_config_namevalues);
 
@@ -138,15 +139,12 @@ class ContinuousBatchingPipeline::Impl {
         return m_pipeline_metrics;
     }
 
-    std::vector<std::string> get_model_configuration() {
-        return m_model_config_namevalues;
-    }
-
-    void print_model_configuration() {
-        std::cout << "Loaded model configuration:" << std::endl;
+    std::string get_model_configuration_string() {
+        std::string print_values = "";
         for( auto prop : m_model_config_namevalues) {
-            std::cout << "\t" << prop << std::endl;
+            print_values = print_values + "\t" + prop + "\n";
         }
+        return print_values;
     }
 
     ov::genai::Tokenizer get_tokenizer() {
@@ -429,15 +427,11 @@ PipelineMetrics ContinuousBatchingPipeline::get_metrics() const{
     return m_impl->get_metrics();
 }
 
-std::vector<std::string> ContinuousBatchingPipeline::get_model_configuration() {
-    return m_impl->get_model_configuration();
-}
-
-void ContinuousBatchingPipeline::print_model_configuration() {
-    return m_impl->print_model_configuration();
+std::string ContinuousBatchingPipeline::get_model_configuration_string() {
+    return m_impl->get_model_configuration_string();
 }
 
-GenerationHandle ContinuousBatchingPipeline::add_request(uint64_t request_id, std::string prompt, ov::genai::GenerationConfig sampling_params) {
+GenerationHandle ContinuousBatchingPipeline::add_request(uint64_t request_id, const std::string& prompt, const ov::genai::GenerationConfig& sampling_params) {
     return m_impl->add_request(request_id, prompt, sampling_params);
 }
 

diff --git a/src/cpp/src/debug_utils.hpp b/src/cpp/src/debug_utils.hpp
@@ -33,46 +33,3 @@ void print_tensor(std::string name, ov::Tensor tensor) {
         print_array(tensor.data<bool>(), tensor.get_size());
     }
 }
-
-std::string join(const std::vector<std::string>& listOfStrings, const std::string delimiter) {
-    std::stringstream ss;
-    auto it = listOfStrings.cbegin();
-    if (it == listOfStrings.end()) {
-        return "";
-    }
-    for (; it != (listOfStrings.end() - 1); ++it) {
-        ss << *it << delimiter;
-    }
-    if (it != listOfStrings.end()) {
-        ss << *it;
-    }
-    return ss.str();
-}
-
-template <typename PropertyExtractor>
-static void read_properties(PropertyExtractor&& property_extractor, std::vector<std::string>& output_configuration_values) {
-    auto key = std::string("SUPPORTED_PROPERTIES");  // ov::supported_properties;
-    std::vector<ov::PropertyName> supported_config_keys;
-    try {
-        ov::Any value = property_extractor(key);
-        supported_config_keys = value.as<std::vector<ov::PropertyName>>();
-    } catch (...) {
-        std::cout << "Exception thrown from OpenVINO when requesting model property: " << key << std::endl;
-        return;
-    }
-
-    for (auto& key : supported_config_keys) {
-        if (key == "SUPPORTED_PROPERTIES")
-            continue;
-        std::string value;
-        try {
-            ov::Any param_value = property_extractor(key);
-            value = param_value.as<std::string>();
-        } catch (...) {
-            std::cout << "WARNING: Exception thrown from OpenVINO when requesting model property: " << key << std::endl;
-            continue;
-        }
-        output_configuration_values.emplace_back(join({key, value}, ": "));
-    }
-    std::sort(output_configuration_values.begin(), output_configuration_values.end());
-}
diff --git a/src/cpp/src/utils.cpp b/src/cpp/src/utils.cpp
@@ -185,6 +185,21 @@ ov::genai::OptionalGenerationConfig get_config_from_map(const ov::AnyMap& config
         return std::nullopt;
 }
 
+std::string join(const std::vector<std::string>& listOfStrings, const std::string delimiter) {
+    std::stringstream ss;
+    auto it = listOfStrings.cbegin();
+    if (it == listOfStrings.end()) {
+        return "";
+    }
+    for (; it != (listOfStrings.end() - 1); ++it) {
+        ss << *it << delimiter;
+    }
+    if (it != listOfStrings.end()) {
+        ss << *it;
+    }
+    return ss.str();
+}
+
 }  // namespace utils
 }  // namespace genai
 }  // namespace ov
diff --git a/src/cpp/src/utils.hpp b/src/cpp/src/utils.hpp
@@ -76,6 +76,28 @@ ov::genai::StreamerVariant get_streamer_from_map(const ov::AnyMap& config_map);
 
 ov::genai::OptionalGenerationConfig get_config_from_map(const ov::AnyMap& config_map);
 
+std::string join(const std::vector<std::string>& listOfStrings, const std::string delimiter);
+
+template <typename PropertyExtractor>
+static void read_properties(PropertyExtractor&& property_extractor, std::vector<std::string>& output_configuration_values) {
+    auto key = std::string("SUPPORTED_PROPERTIES");  // ov::supported_properties;
+    std::vector<ov::PropertyName> supported_config_keys;
+
+    ov::Any value = property_extractor(key);
+    supported_config_keys = value.as<std::vector<ov::PropertyName>>();
+
+    for (auto& key : supported_config_keys) {
+        if (key == "SUPPORTED_PROPERTIES")
+            continue;
+        std::string value;
+        ov::Any param_value = property_extractor(key);
+        value = param_value.as<std::string>();
+
+        output_configuration_values.emplace_back(join({key, value}, ": "));
+    }
+    std::sort(output_configuration_values.begin(), output_configuration_values.end());
+}
+
 }  // namespace utils
 }  // namespace genai
 }  // namespace ov