configuring genai model deployment with model readonly filesystem #4139
There are no files selected for viewing
This file contains hidden or 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 hidden or 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 hidden or 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 hidden or 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 hidden or 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 hidden or 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
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -52,6 +52,20 @@ | |||||
| #endif | ||||||
| namespace ovms { | ||||||
|
|
||||||
| static std::string s_inMemoryGraphContent; | ||||||
|
Collaborator
There was a problem hiding this comment. Why |
||||||
|
|
||||||
| bool GraphExport::hasInMemoryGraphContent() { | ||||||
| return !s_inMemoryGraphContent.empty(); | ||||||
| } | ||||||
|
|
||||||
| const std::string& GraphExport::getInMemoryGraphContent() { | ||||||
| return s_inMemoryGraphContent; | ||||||
| } | ||||||
|
|
||||||
| void GraphExport::clearInMemoryGraphContent() { | ||||||
| s_inMemoryGraphContent.clear(); | ||||||
| } | ||||||
|
|
||||||
| static const std::string OVMS_VERSION_GRAPH_LINE = std::string("# File created with: ") + PROJECT_NAME + std::string(" ") + PROJECT_VERSION + std::string("\n"); | ||||||
|
|
||||||
| static std::string constructModelsPath(const std::string& modelPath, const std::optional<std::string>& ggufFilenameOpt) { | ||||||
|
|
@@ -91,22 +105,26 @@ std::string GraphExport::getDraftModelDirectoryPath(const std::string& directory | |||||
| } \ | ||||||
| auto pluginConfigOpt = std::get<std::optional<std::string>>(pluginConfigOrStatus) | ||||||
|
|
||||||
| static Status createPbtxtFile(const std::string& directoryPath, const std::string& pbtxtContent, bool writeToFile) { | ||||||
| #if (MEDIAPIPE_DISABLE == 0) | ||||||
| ::mediapipe::CalculatorGraphConfig config; | ||||||
| SPDLOG_TRACE("Generated pbtxt\n: {}", pbtxtContent); | ||||||
|
Collaborator
There was a problem hiding this comment.
Suggested change
|
||||||
| bool success = ::google::protobuf::TextFormat::ParseFromString(pbtxtContent, &config); | ||||||
| if (!success) { | ||||||
| SPDLOG_ERROR("Created graph config file couldn't be parsed - check used task parameters values."); | ||||||
| return StatusCode::MEDIAPIPE_GRAPH_CONFIG_FILE_INVALID; | ||||||
| } | ||||||
| #endif | ||||||
| if (!writeToFile) { | ||||||
| s_inMemoryGraphContent = pbtxtContent; | ||||||
| return StatusCode::OK; | ||||||
| } | ||||||
| // clang-format on | ||||||
| std::string fullPath = FileSystem::joinPath({directoryPath, "graph.pbtxt"}); | ||||||
| return FileSystem::createFileOverwrite(fullPath, pbtxtContent); | ||||||
|
Comment on lines
+118
to
124
There was a problem hiding this comment. When writeToFile==false, s_inMemoryGraphContent is populated, but there is no corresponding clearing when writeToFile==true (or before generating a new graph). This means a previous in-memory graph can persist and later be picked up by GraphExport::hasInMemoryGraphContent(), causing unrelated runs/tests to load stale graph content instead of reading graph.pbtxt from disk. Clear the in-memory content whenever you generate/write a file-based graph (and/or at the start of each server startup) so the in-memory fallback cannot leak across runs.
Collaborator
Author
There was a problem hiding this comment. it is added in https://github.com/openvinotoolkit/model_server/pull/4139/changes#diff-d0ecb51621381529aa2b31bb04fecee4a1b1b7dc0be53978c4878f2c4d4bfe1fR503 |
||||||
| } | ||||||
|
|
||||||
| static Status createTextGenerationGraphTemplate(const std::string& directoryPath, const HFSettingsImpl& hfSettings, bool writeToFile) { | ||||||
| if (!std::holds_alternative<TextGenGraphSettingsImpl>(hfSettings.graphSettings)) { | ||||||
| SPDLOG_ERROR("Graph options not initialized for text generation."); | ||||||
| return StatusCode::INTERNAL_ERROR; | ||||||
|
|
@@ -198,10 +216,10 @@ static Status createTextGenerationGraphTemplate(const std::string& directoryPath | |||||
| } | ||||||
| } | ||||||
| })"; | ||||||
| return createPbtxtFile(directoryPath, oss.str(), writeToFile); | ||||||
| } | ||||||
|
|
||||||
| static Status createRerankGraphTemplate(const std::string& directoryPath, const HFSettingsImpl& hfSettings, bool writeToFile) { | ||||||
| if (!std::holds_alternative<RerankGraphSettingsImpl>(hfSettings.graphSettings)) { | ||||||
| SPDLOG_ERROR("Graph options not initialized for reranking."); | ||||||
| return StatusCode::INTERNAL_ERROR; | ||||||
|
|
@@ -242,10 +260,10 @@ node { | |||||
| } | ||||||
| } | ||||||
| })"; | ||||||
| return createPbtxtFile(directoryPath, oss.str(), writeToFile); | ||||||
| } | ||||||
|
|
||||||
| static Status createEmbeddingsGraphTemplate(const std::string& directoryPath, const HFSettingsImpl& hfSettings, bool writeToFile) { | ||||||
| if (!std::holds_alternative<EmbeddingsGraphSettingsImpl>(hfSettings.graphSettings)) { | ||||||
| SPDLOG_ERROR("Graph options not initialized for embeddings."); | ||||||
| return StatusCode::INTERNAL_ERROR; | ||||||
|
|
@@ -289,10 +307,10 @@ node { | |||||
| oss << R"(} | ||||||
| } | ||||||
| })"; | ||||||
| return createPbtxtFile(directoryPath, oss.str(), writeToFile); | ||||||
| } | ||||||
|
|
||||||
| static Status createTextToSpeechGraphTemplate(const std::string& directoryPath, const HFSettingsImpl& hfSettings, bool writeToFile) { | ||||||
| if (!std::holds_alternative<TextToSpeechGraphSettingsImpl>(hfSettings.graphSettings)) { | ||||||
| SPDLOG_ERROR("Graph options not initialized for speech generation."); | ||||||
| return StatusCode::INTERNAL_ERROR; | ||||||
|
|
@@ -339,11 +357,15 @@ node { | |||||
| } | ||||||
| #endif | ||||||
| // clang-format on | ||||||
| if (!writeToFile) { | ||||||
| s_inMemoryGraphContent = oss.str(); | ||||||
| return StatusCode::OK; | ||||||
| } | ||||||
| std::string fullPath = FileSystem::joinPath({directoryPath, "graph.pbtxt"}); | ||||||
| return FileSystem::createFileOverwrite(fullPath, oss.str()); | ||||||
| } | ||||||
|
|
||||||
| static Status createSpeechToTextGraphTemplate(const std::string& directoryPath, const HFSettingsImpl& hfSettings, bool writeToFile) { | ||||||
| if (!std::holds_alternative<SpeechToTextGraphSettingsImpl>(hfSettings.graphSettings)) { | ||||||
| SPDLOG_ERROR("Graph options not initialized for speech to text."); | ||||||
| return StatusCode::INTERNAL_ERROR; | ||||||
|
|
@@ -405,11 +427,15 @@ node { | |||||
| } | ||||||
| #endif | ||||||
| // clang-format on | ||||||
| if (!writeToFile) { | ||||||
| s_inMemoryGraphContent = oss.str(); | ||||||
| return StatusCode::OK; | ||||||
| } | ||||||
| std::string fullPath = FileSystem::joinPath({directoryPath, "graph.pbtxt"}); | ||||||
| return FileSystem::createFileOverwrite(fullPath, oss.str()); | ||||||
| } | ||||||
|
|
||||||
| static Status createImageGenerationGraphTemplate(const std::string& directoryPath, const HFSettingsImpl& hfSettings, bool writeToFile) { | ||||||
| if (!std::holds_alternative<ImageGenerationGraphSettingsImpl>(hfSettings.graphSettings)) { | ||||||
| SPDLOG_ERROR("Graph options not initialized for image generation."); | ||||||
| return StatusCode::INTERNAL_ERROR; | ||||||
|
|
@@ -489,13 +515,13 @@ node: { | |||||
| } | ||||||
| )"; | ||||||
| // clang-format on | ||||||
| return createPbtxtFile(directoryPath, oss.str(), writeToFile); | ||||||
| } | ||||||
|
|
||||||
| GraphExport::GraphExport() { | ||||||
| } | ||||||
|
|
||||||
| Status GraphExport::createServableConfig(const std::string& directoryPath, const HFSettingsImpl& hfSettings, bool writeToFile) { | ||||||
| if (directoryPath.empty()) { | ||||||
| SPDLOG_ERROR("Directory path empty: {}", directoryPath); | ||||||
| return StatusCode::PATH_INVALID; | ||||||
|
|
@@ -518,17 +544,17 @@ Status GraphExport::createServableConfig(const std::string& directoryPath, const | |||||
| } | ||||||
| } | ||||||
| if (hfSettings.task == TEXT_GENERATION_GRAPH) { | ||||||
| return createTextGenerationGraphTemplate(directoryPath, hfSettings, writeToFile); | ||||||
| } else if (hfSettings.task == EMBEDDINGS_GRAPH) { | ||||||
| return createEmbeddingsGraphTemplate(directoryPath, hfSettings, writeToFile); | ||||||
| } else if (hfSettings.task == RERANK_GRAPH) { | ||||||
| return createRerankGraphTemplate(directoryPath, hfSettings, writeToFile); | ||||||
| } else if (hfSettings.task == IMAGE_GENERATION_GRAPH) { | ||||||
| return createImageGenerationGraphTemplate(directoryPath, hfSettings, writeToFile); | ||||||
| } else if (hfSettings.task == TEXT_TO_SPEECH_GRAPH) { | ||||||
| return createTextToSpeechGraphTemplate(directoryPath, hfSettings, writeToFile); | ||||||
| } else if (hfSettings.task == SPEECH_TO_TEXT_GRAPH) { | ||||||
| return createSpeechToTextGraphTemplate(directoryPath, hfSettings, writeToFile); | ||||||
| } else if (hfSettings.task == UNKNOWN_GRAPH) { | ||||||
| SPDLOG_ERROR("Graph options not initialized."); | ||||||
| return StatusCode::INTERNAL_ERROR; | ||||||
|
|
||||||
This file contains hidden or 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 hidden or 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 hidden or 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 hidden or 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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -26,6 +26,7 @@ | |
|
|
||
| #include "../execution_context.hpp" | ||
| #include "src/filesystem/filesystem.hpp" | ||
| #include "src/graph_export/graph_export.hpp" | ||
| #include "src/metrics/metric.hpp" | ||
| #include "../model_metric_reporter.hpp" | ||
| #include "../ov_utils.hpp" | ||
|
|
@@ -60,6 +61,13 @@ const tensor_map_t MediapipeGraphDefinition::getOutputsInfo() const { | |
| } | ||
|
|
||
| Status MediapipeGraphDefinition::validateForConfigFileExistence() { | ||
| if (GraphExport::hasInMemoryGraphContent()) { | ||
| const std::string& content = GraphExport::getInMemoryGraphContent(); | ||
|
Collaborator
There was a problem hiding this comment. Is it global? Not tied to any specific servable object?
Collaborator
Author
There was a problem hiding this comment. yes, assumption is that only one graph can be stored in memory. using multiple models requires creating graph.pbtxt in model folder via --pull command. |
||
| this->chosenConfig = content; | ||
| this->mgconfig.setCurrentGraphPbTxtMD5(ovms::FileSystem::getStringMD5(content)); | ||
| SPDLOG_LOGGER_DEBUG(modelmanager_logger, "Using in-memory graph content for mediapipe graph definition: {}", this->getName()); | ||
| return StatusCode::OK; | ||
| } | ||
|
Comment on lines
63
to
+70
There was a problem hiding this comment. validateForConfigFileExistence() unconditionally prefers GraphExport in-memory content whenever it is set. Since that content is global and not scoped to a particular graph/model, this can cause the wrong graph to be loaded (e.g., if a previous startup/test left in-memory content set) and will also bypass filesystem-based reload semantics. Consider scoping the in-memory graph to the specific startup flow (or at least clearing it after it has been consumed) so normal file-based graphs aren’t shadowed. |
||
| std::ifstream ifs(this->mgconfig.getGraphPath()); | ||
| if (!ifs.is_open()) { | ||
| SPDLOG_LOGGER_ERROR(modelmanager_logger, "Failed to open mediapipe graph definition: {}, file: {}\n", this->getName(), this->mgconfig.getGraphPath()); | ||
|
|
||
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.
GenAI name but it has nothing to do with OpenVINO GenAI, I dont think its good naming