Skip to content

Conversation

@javier-intel
Copy link

Description

When ep.context_file_path is not provided the model metadata name was missing the "_ctx" postfixed to the model name.

Motivation and Context

Conform to context model documentation

@javier-intel javier-intel requested a review from MayureshV1 June 27, 2025 22:08
@MayureshV1 MayureshV1 requested a review from Copilot June 27, 2025 22:11
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR updates the logic that generates the metadata filename so that when no context file path is provided, the model metadata name correctly includes the _ctx prefix before _metadata.bin.

  • Introduce a name_append variable to hold the suffix.
  • When session_context_.so_context_file_path is empty, set suffix to "_ctx_metadata.bin" instead of "_metadata.bin".
Comments suppressed due to low confidence (2)

onnxruntime/core/providers/openvino/openvino_execution_provider.cc:193

  • [nitpick] Consider renaming name_append to something more descriptive like metadata_suffix to clarify its purpose.
      std::string name_append{"_metadata.bin"};

onnxruntime/core/providers/openvino/openvino_execution_provider.cc:198

  • Add unit tests for both branches (with and without so_context_file_path) to verify that the metadata filename ends with _metadata.bin and _ctx_metadata.bin respectively.
      auto metadata_filename = metadata_file_path.stem().string() + name_append;

Copy link

@MayureshV1 MayureshV1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM ! Fixes the CreateSession Scenario where the metadata_bin file was not utilzing the ctx model name.

@MayureshV1 MayureshV1 merged commit dfc8ff0 into ovep-develop Jun 27, 2025
6 of 8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants