Skip to content

[Bugfix] Fix Fish Speech startup on vLLM 0.18.0 - missing architectures#2166

Closed
linyueqian wants to merge 2 commits into
vllm-project:mainfrom
linyueqian:fix/fish-speech-architectures
Closed

[Bugfix] Fix Fish Speech startup on vLLM 0.18.0 - missing architectures#2166
linyueqian wants to merge 2 commits into
vllm-project:mainfrom
linyueqian:fix/fish-speech-architectures

Conversation

@linyueqian
Copy link
Copy Markdown
Collaborator

@linyueqian linyueqian commented Mar 25, 2026

Summary

Fix Fish Speech (and other models without architectures in config.json) failing to start on vLLM 0.18.0.

vLLM 0.18.0 now requires architectures in the model config. Fish Speech s2-pro's config.json doesn't have this field, but the stage config already specifies model_arch. This fix auto-injects model_arch into hf_overrides before ModelConfig creation.

Test plan

# Before: crashes with "No model architectures are specified"
# After: starts successfully
python examples/offline_inference/fish_speech/end2end.py \
    --model fishaudio/s2-pro --text "Hello test"

Tested with s2-pro on A100, offline inference produces valid 1.02s audio @ 44.1kHz.

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 0fe68c8da6

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment thread vllm_omni/engine/arg_utils.py Outdated
from vllm.transformers_utils.config import get_config

try:
hf_cfg = get_config(self.model, trust_remote_code=self.trust_remote_code)
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Avoid loading Hugging Face config twice during startup

This new pre-check calls get_config(...) before super().create_model_config(), and EngineArgs.create_model_config() will load the HF config again when constructing the vLLM ModelConfig. For stages that set model_arch and have no hf_overrides, this adds an extra remote/local config load per stage, which increases startup time and can introduce avoidable transient fetch failures for multi-stage pipelines. Prefer reusing a single loaded config or injecting/merging architectures without a second fetch path.

Useful? React with 👍 / 👎.

@linyueqian
Copy link
Copy Markdown
Collaborator Author

@Sy0307 is this patch necessary? strange that i need to add this for fish speech.

When a model's config.json lacks the `architectures` field (e.g.
Fish Speech s2-pro), vLLM 0.18.0's ModelConfig.__init__ raises
"No model architectures are specified". The stage config already
specifies `model_arch` but it was only used after ModelConfig
creation, which is too late.

Now injects `model_arch` via `hf_overrides` before calling
super().create_model_config(). No extra config load needed —
hf_overrides merges cleanly when architectures already exists.

Tested with Fish Speech s2-pro and Qwen3-TTS.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Signed-off-by: linyueqian <linyueqian@outlook.com>
@linyueqian linyueqian force-pushed the fix/fish-speech-architectures branch from 0fe68c8 to dd0f833 Compare March 25, 2026 04:10
@gcanlin
Copy link
Copy Markdown
Collaborator

gcanlin commented Mar 25, 2026

Please see #1898. Looks like we don't need it?

@Sy0307
Copy link
Copy Markdown
Contributor

Sy0307 commented Mar 25, 2026

@Sy0307 is this patch necessary? strange that i need to add this for fish speech.

When I am checking out vllm 0.18.0, the same issue occured. This PR is necessary for me.

@gcanlin
Copy link
Copy Markdown
Collaborator

gcanlin commented Mar 25, 2026

cc @princepride

@princepride
Copy link
Copy Markdown
Collaborator

I will check it later.

@princepride
Copy link
Copy Markdown
Collaborator

I believe this fix may not solve the real problem, because when we already give the model's architecture, there is no sense we need hf_override to override it.

@princepride
Copy link
Copy Markdown
Collaborator

The model_arch in the YAML does not take effect during vLLM's base ModelConfig.init phase.

Timing issue:

  1. OmniEngineArgs picks up self.model_arch = "FishSpeechSlowARForConditionalGeneration" from the YAML.
  2. It calls super().create_model_config() → vLLM's ModelConfig.init runs.
  3. ModelConfig.init reads the architecture from hf_config.architectures → at this point model_arch has not been used yet.
  4. If the HF config.json has no architectures field → crash.
  5. OmniModelConfig.from_vllm_model_config(model_arch=...) is never reached.

model_arch only takes effect after step 5 (via the OmniModelConfig.architectures property override + the hf_config.architectures write-back), but because the fishaudio/s2-pro model's config.json is non-standard and lacks an architectures field, the program already crashes at step 4.

Copy link
Copy Markdown
Collaborator

@princepride princepride left a comment

Choose a reason for hiding this comment

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

LGTM

@linyueqian linyueqian added the ready label to trigger buildkite CI label Mar 25, 2026
@linyueqian linyueqian enabled auto-merge (squash) March 25, 2026 15:39
@linyueqian
Copy link
Copy Markdown
Collaborator Author

Closing in favor of #2178 which was already merged with an equivalent fix.

@linyueqian linyueqian closed this Mar 25, 2026
auto-merge was automatically disabled March 25, 2026 20:02

Pull request was closed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready label to trigger buildkite CI

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants