Skip to content

Conversation

dtrawins
Copy link
Collaborator

… models

🛠 Summary

🧪 Checklist

  • Unit tests added.
  • The documentation updated.
  • Change follows security best practices.
    ``

Copy link
Contributor

@Copilot 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 documentation and improves the logic for determining when to use git clone vs optimum-cli for downloading models from Hugging Face. The changes make the documentation clearer about model conversion requirements and relax the conditions for cloning models to be based on naming patterns rather than organization names.

  • Relaxes the condition for git clone to use pattern matching for OpenVINO models instead of organization-based detection
  • Updates documentation to clarify the difference between pre-configured IR models and models requiring conversion
  • Updates test cases to use more generic model names

Reviewed Changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.

File Description
src/cli_parser.cpp Replaces isOptimumCliDownload function with pattern-based detection for OpenVINO models
src/test/ovmsconfig_test.cpp Updates test model names to use "Unknown" organization instead of "NonOpenVINO"
docs/pull_optimum_cli.md Restructures documentation for clarity and updates docker image references
docs/pull_hf_models.md Clarifies distinction between pre-configured and conversion-required models

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

if (isOptimumCliDownload(serverSettings.hfSettings.sourceModel, hfSettings.ggufFilename)) {
// Cloning the repository is allowed only for OpenVINO models determined by the name pattern
// Other models will be downloaded and converted using optimum-cli or just downloaded as GGUF
std::string lowerSourceModel = toLower(serverSettings.hfSettings.sourceModel);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Replace logic inside isOptimumCliDownload. This will break --draft model pull

Copy link
Collaborator

Choose a reason for hiding this comment

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

We should have also test for what acceptin "-ov", "_ov" inside if we want to change this behavior. Are those official repositories of OpenVINO? Wouldn't it make sense to have separate switch to choose which downloader we should use instead of relying on organization?
User then could use optimum/libgit2 regardless if sourcemodel name sticks to some arbitral convention.

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.

2 participants