Repository navigation
[Bugfix][Frontend] Parse VLLM_USE_MODELSCOPE via vllm.envs - #5428
Merged
Gaohan123 merged 2 commits intoJul 27, 2026
Merged
Conversation
omni_snapshot_download() read the raw environment variable, so every non-empty value was truthy. Setting VLLM_USE_MODELSCOPE=0 (or False/no/off) to explicitly opt out therefore selected the ModelScope download path, which raises ModuleNotFoundError on a default install because modelscope is not a vLLM-Omni dependency. vLLM enables the flag only for the literal string "true" (case-insensitive), and vllm_omni/model_executor/model_loader/weight_utils.py already reads it through vllm.envs. Route this call site through vllm.envs too so both agree. Signed-off-by: Yash Jain <yashjainjain1704@gmail.com>
yashkgp
requested review from
alex-jw-brooks,
tzhouam and
yenuo26
as code owners
July 26, 2026 21:55
|
Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits. |
Gaohan123
enabled auto-merge (squash)
July 27, 2026 04:00
nagisa-kunhah
pushed a commit
to nagisa-kunhah/vllm-omni
that referenced
this pull request
Jul 30, 2026
…ect#5428) Signed-off-by: Yash Jain <yashjainjain1704@gmail.com>
khairulkabir1661
pushed a commit
to khairulkabir1661/vllm-omni
that referenced
this pull request
Sep 25, 2026
…ect#5428) Signed-off-by: Yash Jain <yashjainjain1704@gmail.com>
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Purpose
Fixes #5427.
omni_snapshot_download()checkedVLLM_USE_MODELSCOPEfor truthiness instead ofparsing it, so every non-empty value enabled ModelScope, including
0,False,false,noandoff.vLLM only treats the literal string
"true"(case-insensitive) as enabled, seevllm/envs.py@ v0.25.0:omni_snapshot_download()is the first call inOmniBase.__init__, so withVLLM_USE_MODELSCOPE=0both offlineOmni(...)andvllm-omni servetook theModelScope branch.
modelscopeis not a vLLM-Omni dependency (it is onlyinstalled in
docker/Dockerfile.xpu), so a default install failed at startup withModuleNotFoundError: No module named 'modelscope'. Where it is installed,weights were fetched from ModelScope despite the opt-out.
vllm_omni/model_executor/model_loader/weight_utils.py#L9already reads the flagthrough
vllm.envs, so the two paths disagreed inside one process. This routesthe call site through
vllm.envsas well.VLLM_USE_MODELSCOPE=truebehaves asbefore.
Test Plan
Added
tests/entrypoints/test_omni_snapshot_download.py, markedcore_modelandcpuso the existing L1Simple · Other Testlane in.buildkite/cuda/test-ready.ymlcollects it, no CI wiring changes needed. Itstubs the ModelScope module into
sys.modules, otherwise the wrong branch raisesModuleNotFoundErrorinstead of being observable, then asserts which backendomni_snapshot_download()selects for each flag value.pytest -sv tests/entrypoints/test_omni_snapshot_download.py -m 'core_model and cpu'Repro for the original bug:
vLLM Version: v0.25.0
vLLM-Omni Commit: 2bb6189
Test Result
On
main, 6 of the 10 cases fail:With the fix:
ruff check,ruff format --checkandtools/pre_commit/check_test_marks.pyareclean on both changed files.
I do not have a CUDA box, so that pytest run was done in a CPU sandbox that pairs
the real
vllm/envs.pyfrom v0.25.0 with the source ofomni_snapshot_download()taken from this branch, and from
mainfor the "before" numbers. The test file isthe same one in this PR. The full
tests/entrypoints/suite needs CI.