Skip to content

Feature/add -ms option for modelscope - #22279

Closed
yrk111222 wants to merge 8 commits into
ggml-org:masterfrom
yrk111222:feature/add-ms-option
Closed

Feature/add -ms option for modelscope#22279
yrk111222 wants to merge 8 commits into
ggml-org:masterfrom
yrk111222:feature/add-ms-option

Conversation

@yrk111222

Copy link
Copy Markdown

Overview

This PR introduces ModelScope integration for model downloading and loading.
Related PR: #20941
#21985

Key changes:

Added new cli arguments: -ms (ModelScope repo ID)
Added corresponding environment variables support for seamless configuration.
Enabled community model downloading via MODEL_ENDPOINT (defaults to https://modelscope.cn/), allowing flexible usage with or without explicit endpoint specification.

Usage examples:

# 1. Download via ModelScope ID (uses default or MODEL_ENDPOINT)
./build/bin/llama-cli -ms Qwen/Qwen3-0.6B-GGUF:Q8_0 -p "hello"
or
MODEL_ENDPOINT=https://modelscope.cn/ ./build/bin/llama-cli -ms Qwen/Qwen3-0.6B-GGUF:Q8_0 -p "hello"

# 2. Specify file and token explicitly (or set 'MS_TOKEN' env)
./build/bin/llama-cli -ms <repo> -hff <file> -hft <token> -p "hello"

#3. For vision models, the mmproj usage with `-ms` is identical to `-hf`.

Additional information

Requirements

@yrk111222
yrk111222 requested a review from a team as a code owner April 23, 2026 10:31
@ggml-gh-bot

ggml-gh-bot Bot commented Apr 23, 2026

Copy link
Copy Markdown

Hi @yrk111222, thanks for your contribution!

Per our contribution guidelines, the automated PR checker found the following issue(s) that need your attention:

  • Multiple open PRs from a new contributor: We limit new contributors (those without a previously merged PR) to 1 open PR at a time. You currently have 2 open PRs.

Please note that maintainers reserve the right to make final decisions on PRs. If you believe there is a mistake, please comment below.

@yrk111222

Copy link
Copy Markdown
Author

Hi @yrk111222, thanks for your contribution!

Per our contribution guidelines, the automated PR checker found the following issue(s) that need your attention:

  • Multiple open PRs from a new contributor: We limit new contributors (those without a previously merged PR) to 1 open PR at a time. You currently have 2 open PRs.

Please note that maintainers reserve the right to make final decisions on PRs. If you believe there is a mistake, please comment below.

I have closed the previous PR #21985 to comply with the single-PR rule for new contributors.

@yrk111222
yrk111222 force-pushed the feature/add-ms-option branch from 31c96a5 to ccd52dc Compare April 28, 2026 01:55
@yrk111222

Copy link
Copy Markdown
Author

Hi @ngxson @angt , here is a new PR adding ModelScope -ms support. Please take a look when time permits. Thanks!🙏

@angt

angt commented Apr 28, 2026

Copy link
Copy Markdown
Member

Hi @yrk111222, thanks for the update.
However, the PR feels particularly wrong because it duplicates a lot of code that shouldn’t be 👀

@yrk111222
yrk111222 force-pushed the feature/add-ms-option branch from ccd52dc to 9919f03 Compare May 6, 2026 08:31
@yrk111222

Copy link
Copy Markdown
Author

Hi @angt — thanks for the earlier review. I reworked this PR to remove duplication by switching to common_download_file_single and keeping only the ModelScope-specific bits (dropped the custom progress bar/download loop, extracted shared helpers).
Could you please take another look when you have a moment?
If anything else comes up, feel free to ping me anytime — thanks!😊

@yrk111222

Copy link
Copy Markdown
Author

Hi @angt @ngxson , hope you’re having a great day! 😊
Just a gentle follow-up — I’ve refactored the PR to remove the duplicated code as you suggested.
Whenever you have a moment, could you kindly take another look?
Please also let me know if there’s anything else that needs improvement or if you have any other concerns — I’d be happy to adjust further.
Thanks so much for your time and guidance!

@yrk111222
yrk111222 force-pushed the feature/add-ms-option branch 3 times, most recently from d3262d5 to 1d4359f Compare May 20, 2026 03:22
@angt

angt commented May 21, 2026

Copy link
Copy Markdown
Member

Hey @yrk111222 thanks for the update, there is still a duplication of the download code 👀

@yrk111222
yrk111222 force-pushed the feature/add-ms-option branch from 1d4359f to e5c1eba Compare May 21, 2026 09:03
@Leonard-Li777

Copy link
Copy Markdown

Expect the feature to be merged and released immediately

@yrk111222
yrk111222 force-pushed the feature/add-ms-option branch 7 times, most recently from 9e29477 to 3419477 Compare May 22, 2026 10:13
@yrk111222
yrk111222 force-pushed the feature/add-ms-option branch from 3419477 to 7d7edbb Compare May 25, 2026 08:54
@yrk111222

Copy link
Copy Markdown
Author

Hi @angt, thanks for your feedback. Since your last review, I've further reduced duplication.
Could you take another look when you have time? Thanks!🙏

@yrk111222

Copy link
Copy Markdown
Author

Hi @angt. Friendly ping — could you take a look when you get a chance? Thanks! 😊

@yingdachen

Copy link
Copy Markdown

this has been pending for a while, could you please follow up and take a look when you got a chance? @angt @Leonard-Li777

thanks a bunch!

@angt

angt commented Jun 9, 2026

Copy link
Copy Markdown
Member

Hey!

We know this feature is requested by users, so we really appreciate the effort. But rn, we are rethinking our overall cache system and how users interact with it. Because of this upcoming redesign, several related PRs are currently on hold, and I apologize for the delay this causes.

At the same time, this PR currently mixes a few different responsibilities. While we acknowledge that the existing codebase is already a bit messy, we want to avoid adding more technical debt before the redesign. All the logic for downloading, quantization selection, and filtering should stay in download.cpp for now. To keep things future-proof, ms-cache.cpp needs to follow the pattern of hf-cache.cpp more closely.

@yingdachen

Copy link
Copy Markdown

Hey!

We know this feature is requested by users, so we really appreciate the effort. But rn, we are rethinking our overall cache system and how users interact with it. Because of this upcoming redesign, several related PRs are currently on hold, and I apologize for the delay this causes.

At the same time, this PR currently mixes a few different responsibilities. While we acknowledge that the existing codebase is already a bit messy, we want to avoid adding more technical debt before the redesign. All the logic for downloading, quantization selection, and filtering should stay in download.cpp for now. To keep things future-proof, ms-cache.cpp needs to follow the pattern of hf-cache.cpp more closely.

thanks @angt , we will be working on a revision according to your suggestions.

@yrk111222

Copy link
Copy Markdown
Author

Hello @angt
Thanks for the your detailed feedback! Based on your guidance about following the hf-cache.cpp pattern more closely and keeping download/selection logic in download.cpp, I've restructured the implementation and opened a fresh PR #24716 to keep the history clean.

Closing this one in favor of #24716. Happy to address any further feedback there!

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.

4 participants