Openvino/ep weight sharing #548

sfatimar · 2025-01-25T06:14:29Z

These changes are done to ensure that weight sharing happens between two model using session context option ep_weight_sharing. Key changes introduced in this feature are:

Creating a shared context between two models
Extracting external constant initializers and re labelling them back as inputs to the model to allow weight loading in the direct blob.
Creating EP Context Nodes when Subgraph partitioning is happening.

Motivation and Context

This change was required to ensure that LLM with prefill and kvcache models can use the same share
The change was also required to ensure EP Context nodes can be formed even when model is being subgraph partitioned.

sfatimar · 2025-01-27T06:55:38Z

onnxruntime/core/providers/openvino/backend_manager.cc

+      return "";
+    } else {
+      auto input_type = graph_viewer.GetInputs()[0]->TypeAsProto()->tensor_type().elem_type();
+      if (session_context_.precision == "ACCURACY" &&


Is this taking care of AUTO:GPU, CPU case for Adobe.?

@preetha-intel

is ACCURACY only for GPU? What about NPU

Accuracy is currently enabled for GPU. NPU only F16 precision is enabled from OVEP.
The change here is to get the subgraphs input precision.

onnxruntime/onnxruntime/core/providers/openvino/backends/basic_backend.cc

Line 167 in a6698ce

if (session_context_.precision.find("ACCURACY") != std::string::npos &&

The config for OV ACCURACY mode remains unchanges and it takes care of Adobe case.

sfatimar · 2025-01-27T07:04:45Z

onnxruntime/core/providers/openvino/backend_manager.cc

-  }
  subgraph_context_.subgraph_name = fused_node.Name();
+
+  ptr_stream_t model_stream;


Can we rename this from ptr_stream_t to something more specific like model_stream_t

sfatimar · 2025-01-27T07:05:50Z

onnxruntime/core/providers/openvino/backend_manager.cc

+    std::filesystem::path weight_filename = session_context_.onnx_model_path_name.parent_path();
+    if (sw.external_weight_filename.empty() && !sw.metadata.empty()) {
+      // Reasonable assumption that all metadata entries have the same external file location
+      sw.external_weight_filename = sw.metadata.begin()->second.location;


Is this assumption for the single model or for two seperate models.

sfatimar · 2025-01-27T07:06:41Z

onnxruntime/core/providers/openvino/backend_manager.cc

+    weight_filename /= sw.external_weight_filename;
+    std::ifstream weight_file(weight_filename);
+
+    if (weight_file) {


Jatin to do a coverity check

sfatimar · 2025-01-27T07:07:14Z

onnxruntime/core/providers/openvino/backend_manager.cc

+    std::filesystem::path weight_filename = session_context_.onnx_model_path_name.parent_path();
+    if (sw.external_weight_filename.empty() && !sw.metadata.empty()) {
+      // Reasonable assumption that all metadata entries have the same external file location
+      sw.external_weight_filename = sw.metadata.begin()->second.location;


We are accessing external weight filename without any checks on the path being valid

sfatimar · 2025-01-27T07:09:09Z

onnxruntime/core/providers/openvino/backend_manager.cc

        "To export this model, set disable_dynamic_shapes to False";
    ORT_THROW(exception_str);
  }



This was MSFT requirement to ensure that cache file dir and ep context model blob file path should be mapped ?
@preetha-intel is there a functionality break here

This check ensures that only epctx model or OV cache files will be generated.

onnxruntime/onnxruntime/core/providers/openvino/backends/basic_backend.cc

Line 307 in a6698ce

if (!session_context_.cache_dir.empty() && !session_context_.so_context_enable) {

ep.context_enable takes the higher precedence

sfatimar · 2025-01-27T07:10:20Z

onnxruntime/core/providers/openvino/backend_manager.cc

+    if (blob_filename.empty()) {
+      blob_filename = session_context_.onnx_model_path_name;
+    }
+    const auto name = graph_body_viewer.ModelPath().stem().string() + "_" + subgraph_context_.subgraph_name;


There are no checks on the path and parent path to see if we are not pointing to restricted locations. Please talk to Ankit and Vishnu on filepath restrictions

sfatimar · 2025-01-27T07:14:30Z

onnxruntime/core/providers/openvino/backend_utils.cc

 #include <fstream>
 #include <utility>

+#include <filesystem>


@saurabhkale17 please run cpplint to ensure this issue does not occur with MSFT CI Pipelines

sfatimar · 2025-01-27T07:16:31Z

onnxruntime/core/providers/openvino/backend_utils.cc

 }

+ov::element::Type GetOpenVINOElementType(ONNX_NAMESPACE::TensorProto_DataType dt) {
+  static std::unordered_map<ONNX_NAMESPACE::TensorProto_DataType, ov::element::Type> map{


@saurabhkale17 can you take care of this cpplint issue

sfatimar · 2025-01-27T09:37:14Z

onnxruntime/core/providers/openvino/backend_utils.cc

+    while (!stream.eof()) {
+      SharedContext::SharedWeights::Metadata::Key key;
+      SharedContext::SharedWeights::Metadata::Value value;
+      stream >> key.name;


how do we prevent overflow here

sfatimar · 2025-01-27T09:44:55Z

onnxruntime/core/providers/openvino/openvino_provider_factory.cc

-    }
-    if (provider_options_map.find("device_id") != provider_options_map.end()) {
-      std::string dev_id = provider_options_map.at("device_id").c_str();
+


are we removing old provider options that might show up backward compatibility issues ?

* Move variables from subgraph to session context for model specific properties * Fix for redundant subgraph creation * Remove unused variable

sfatimar · 2025-01-27T14:48:45Z

Hello I have raised some basic review comments , please determine if they are trivial and raise a JIRA to handle them later.

sfatimar · 2025-01-27T16:34:57Z

LGTM Please take care of review comments going forward.

preetha-intel · 2025-01-28T03:06:51Z

onnxruntime/core/providers/openvino/openvino_provider_factory.cc

+
+    pi.device_type = ParseDeviceType(provider_options, "device_type");
+
+    if (provider_options.contains("device_id")) {


'device_id' is still retained with the deprecation warning

* Rename EP instance context as session_context * Add support for GetEpContextNodes * enable config option for ovep weight sharing * add config option for ovep weight sharing * Refactor the conditional blocks in OVEP for compilation * Convert initializers with external data to graph inputs * create, store and export metadata for ovep weight sharing * fix error handling in weight sharing * fix crash issue while setting up inputs for wai model * pass weight sharing option to OVEP qdq stripping pass * Aligning OVEP variable names to match the session option value they hold * Add plumbing for context sharing plus refactoring around option handling * Store metadata in shared context * fix: fix provider options * create ov tensor from meta data and external data * create ov tensor * Add support for binding weight as input tensors * Fix for mapping subgraph to ov compiled network arguments * Fix for using so_share_ep_contexts without ep.context* flags * Add remote tensor support for NPU weight sharing * Use a single ov::Core copy across OVEP * Decouple provider option cache_dir from session option ep.context_file_path * Add support for serialization and deserialization of metadata to disk * Load blobs from relative path stored in ep_cache_context * Use remote L0 tensors for shared weights * fix linux ci issues * fix ci issues * Fix Windows build failure * Use ifstream to load weights instead of mmaped file * Fix for epctx models made up entirely of OVEP epctx nodes * Limit ov::Core lifetime to that of provider object * Enforce shared tensors cleanup on shutdown * Add support for default device type based on project configuration * fix: Fixed concrete_backend_ pointer double free issue on Linux * Preetha/weight sharing fix (#545) * Move variables from subgraph to session context for model specific properties * Fix for redundant subgraph creation * Remove unused variable --------- Co-authored-by: Javier E. Martinez <[email protected]> Co-authored-by: saurabhkale117 <[email protected]> Co-authored-by: Preetha Veeramalai <[email protected]> Co-authored-by: ankitm3k <[email protected]> Co-authored-by: Eric Crawford <[email protected]>

sfatimar commented Jan 27, 2025

View reviewed changes

ankitm3k force-pushed the ovep-develop branch from 7f2ff4f to a98fc3e Compare January 27, 2025 08:29

javier-intel and others added 20 commits January 27, 2025 14:05

Rename EP instance context as session_context

f6cff34

Add support for GetEpContextNodes

f170c88

enable config option for ovep weight sharing

37cee3f

add config option for ovep weight sharing

409cb47

Refactor the conditional blocks in OVEP for compilation

3b2b7e9

Convert initializers with external data to graph inputs

3949bf5

create, store and export metadata for ovep weight sharing

28c928a

fix error handling in weight sharing

7a89c5a

fix crash issue while setting up inputs for wai model

ad66ae0

pass weight sharing option to OVEP qdq stripping pass

c17b276

Aligning OVEP variable names to match the session option value they hold

5e734f1

Add plumbing for context sharing plus refactoring around option handling

c9fb757

Store metadata in shared context

a78166d

fix: fix provider options

01ac259

create ov tensor from meta data and external data

db075cd

create ov tensor

8209162

Add support for binding weight as input tensors

89ebe8d

Fix for mapping subgraph to ov compiled network arguments

ae408af

Fix for using so_share_ep_contexts without ep.context* flags

ac9c998

Add remote tensor support for NPU weight sharing

6512ec6

ericcraw and others added 6 commits January 27, 2025 14:05

Use ifstream to load weights instead of mmaped file

6f7782c

Fix for epctx models made up entirely of OVEP epctx nodes

f3e4e07

Limit ov::Core lifetime to that of provider object

1d4c16e

Enforce shared tensors cleanup on shutdown

6ee1e16

Add support for default device type based on project configuration

6c108f2

fix: Fixed concrete_backend_ pointer double free issue on Linux

6d1f1cf

ankitm3k force-pushed the openvino/ep-weight-sharing branch from 48ee137 to 6d1f1cf Compare January 27, 2025 08:46

sfatimar commented Jan 27, 2025

View reviewed changes

Preetha/weight sharing fix (#545)

7179a0b

* Move variables from subgraph to session context for model specific properties * Fix for redundant subgraph creation * Remove unused variable

sfatimar merged commit a6698ce into ovep-develop Jan 27, 2025
7 of 17 checks passed

preetha-intel reviewed Jan 28, 2025

View reviewed changes


		pi.device_type = ParseDeviceType(provider_options, "device_type");

		if (provider_options.contains("device_id")) {

Openvino/ep weight sharing #548

Openvino/ep weight sharing #548

Uh oh!

Conversation

sfatimar commented Jan 25, 2025

Motivation and Context

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

preetha-intel Jan 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sfatimar commented Jan 27, 2025

Uh oh!

sfatimar commented Jan 27, 2025

Uh oh!

Uh oh!

preetha-intel Jan 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

preetha-intel Jan 28, 2025 •

edited

Loading

preetha-intel Jan 28, 2025 •

edited

Loading