Skip to content

[ROCm][CI] Gate incompatible HF references on Transformers v5#41532

Open
AndreasKaratzas wants to merge 14 commits into
vllm-project:mainfrom
ROCm:akaratza_lang_mod_hf
Open

[ROCm][CI] Gate incompatible HF references on Transformers v5#41532
AndreasKaratzas wants to merge 14 commits into
vllm-project:mainfrom
ROCm:akaratza_lang_mod_hf

Conversation

@AndreasKaratzas
Copy link
Copy Markdown
Collaborator

This updates generation-test metadata for models whose HF reference path is incompatible with the installed Transformers v5 runtime.

Test groups fixed:

  • mi355_1: Language Models Tests (Standard) for MiniCPM4.
  • mi355_1: Language Models Test (Extended Generation) for HyperCLOVAX.

MiniCPM4:

HyperCLOVAX:

  • The model's remote code imports ROPE_INIT_FUNCTIONS and unconditionally indexes ROPE_INIT_FUNCTIONS[self.rope_type]; for the default case, self.rope_type is "default": https://huggingface.co/naver-hyperclovax/HyperCLOVAX-SEED-Think-14B/blob/main/modeling_hyperclovax.py
  • Transformers v5 docs still list "default" as a valid RoPE type. The incompatibility is narrower: v5 no longer exposes a "default" entry in ROPE_INIT_FUNCTIONS, while v4.57 did. In v5's own Llama code, default RoPE is handled by compute_default_rope_parameters; ROPE_INIT_FUNCTIONS is consulted only when rope_type != "default".
  • The v5 migration guide also notes that rotary embedding configuration now lives under config.rope_parameters, which matches the new v5 model path.

cc @kenroche

Signed-off-by: Andreas Karatzas <akaratza@amd.com>
@AndreasKaratzas AndreasKaratzas marked this pull request as ready for review May 3, 2026 04:15
Copy link
Copy Markdown

@claude claude Bot left a comment

Choose a reason for hiding this comment

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

Claude Code Review

This pull request is from a fork — automated review is disabled. A repository maintainer can comment @claude review to run a one-time review.

@AndreasKaratzas AndreasKaratzas added the rocm Related to AMD ROCm label May 3, 2026
@github-project-automation github-project-automation Bot moved this to Todo in AMD May 3, 2026
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request updates the model testing framework to include model revisions and trust_remote_code flags. It also introduces version constraints for specific models in the registry. Feedback was provided regarding a logic bug where adding a version reason causes tests to be skipped regardless of version validity, and a concern that the specified Transformers version range for MiniCPM4 appears to be incorrect or overly restrictive.

Comment thread tests/models/registry.py
Comment thread tests/models/registry.py
Signed-off-by: Andreas Karatzas <akaratza@amd.com>
@AndreasKaratzas
Copy link
Copy Markdown
Collaborator Author

cc @charlifu

@tjtanaa tjtanaa added the ready ONLY add when PR is ready to merge/full CI is needed label May 4, 2026
@mergify mergify Bot added the multi-modality Related to multi-modality (#4194) label May 5, 2026
@AndreasKaratzas
Copy link
Copy Markdown
Collaborator Author

Notes on the AudioFlamingo3 / MusicFlamingo changes:

  • Transformers 5.5 native processors now require a 1:1 mapping between text entries and audio entries. vLLM's processor path supports multiple audio items inside one prompt, so the AudioFlamingo3 multimodal processor now performs the HF processor's windowing, feature extraction, prompt expansion, and tokenization locally for this single-prompt multi-audio case.
  • MusicFlamingo's native processor no longer returns rote_timestamps. vLLM now derives the same rotary-time timestamps from chunk_counts and the encoded audio sequence length inside the model path, while still accepting rote_timestamps if an older processor provides them.
  • MusicFlamingo prompt expansion now includes the audio BOS/EOS tokens expected by its prompt replacement metadata.
  • transformers_version_reason now only applies when the installed version misses the configured min/max bound. This keeps the AudioFlamingo3/MusicFlamingo "Needs PR 43538" explanation for versions below 5.3.0 without skipping supported 5.5.3 runs.

hmellor

This comment was marked as outdated.

@hmellor hmellor dismissed their stale review May 6, 2026 14:52

misunderstanding

Copy link
Copy Markdown
Contributor

@eustlb eustlb left a comment

Choose a reason for hiding this comment

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

transformers releases
AudioFlamingo3: v5.0.0
MusicFlamingo3: v5.5.0

There should not be any reasons not to use super()._call_hf_processor. Looks like a lot of the confusion here comes from the fact that the vLLM PR was merged before the transformers one on which it depended, and that evolved after the vLLM one was merged.

Such fixes should be addressed in work already started in #39011, but happy to help if needed. @lashahub looping you in here

Signed-off-by: Andreas Karatzas <akaratza@amd.com>
…processing

Signed-off-by: Andreas Karatzas <akaratza@amd.com>
@mergify mergify Bot added the ci/build label May 21, 2026
@AndreasKaratzas
Copy link
Copy Markdown
Collaborator Author

I pushed some more changes (a lot of them actually). The main intent is to stop carrying vLLM-local copies of preprocessing that now belongs to the upstream HF processors. So vLLM had manual chunking, feature extraction, and audio-token expansion logic. With AudioFlamingo3 at transformers v5.0.0 and MusicFlamingo at v5.5.0, that logic should live in HF, while vLLM keeps only the adapter bookkeeping needed for batching, placeholder matching, and model execution.

In audioflamingo3.py, _call_hf_processor now calls super()._call_hf_processor(...). The deleted active-path code was duplicating HF behavior: manually splitting audio into chunks, calling the feature extractor directly, normalizing masks, expanding <sound> tokens, then calling the tokenizer. Current HF AudioFlamingo3Processor.__call__ already does those pieces.

The remaining vLLM logic after super() is:

  • Copy mm_data first so we do not mutate the caller's mapping with pop.
  • Map vLLM's "audios" alias to HF's "audio" argument.
  • Rename HF's input_features_mask to vLLM's feature_attention_mask.
  • Compute chunk_counts, because HF returns flattened audio chunks but vLLM still needs to regroup chunk embeddings back per original audio item.

So chunk_counts is not a replacement for HF preprocessing. It is vLLM batching metadata.

I also removed MusicFlamingo's _call_hf_processor because after the AudioFlamingo3 parent method was fixed, the MusicFlamingo override only duplicated parent behavior and recomputed chunk_counts. MusicFlamingo can inherit the shared AudioFlamingo3 adapter path.

Also regarding MusicFlamingo's _expand_audio_tokens, I think that current HF MusicFlamingoProcessor expands audio placeholders itself, including the MusicFlamingo-specific BOS/audio/EOS pattern. Keeping a second vLLM implementation risks drifting from HF again. vLLM still keeps MusicFlamingo prompt replacement in musicflamingo.py, because vLLM needs to match generated placeholder token IDs to multimodal embedding slots.

The partial_rotary_factor change in musicflamingo.py aligns with current transformers. The previous code used the full head dimension. HF computes dim = int(head_dim * partial_rotary_factor), so I think that vLLM was rotating the wrong dimensionality.

The RoPE forward change from flattened batch_positions to timestamp-derived window_positions also matches current HF. The old implementation made the rotary window axis depend on flattened chunk order. Current HF uses the actual window start timestamp, computes the audio window duration, then derives the window position from that. That matters for chunked audio and for matching single vs batched behavior.

The fp32 RoPE buffer restoration is there because vLLM model dtype/device application can cast non-persistent buffers to bf16. In the HF construction path we compare against, inv_freq and position_angles are built from torch.float, so they remain fp32 under the tested bf16 load path. These buffers feed trig functions; bf16 there is enough to create exact-output drift. _apply lets the module move to the right device, then recreates those RoPE buffers in fp32 on that device.

The cast in _encode_audio_features fixes the ROCm failure where processor output is fp32 but the audio tower conv weights/bias are bf16. ROCm/PyTorch requires input and bias dtypes to match for that conv path. Casting at model ingress is the right boundary: HF processors naturally produce float features, while the vLLM model owns execution dtype/device.

Also, for ROCm, the transcription was wrong yielding a weird "four-on-the-floor" with MusicFlamingo. Direct HF generation matched the original fixture, while vLLM's URL audio path produced a different transcription. The root cause was audio resampling: HF/librosa uses soxr-style resampling, while vLLM was using the default parser resampler. Switching only MusicFlamingo's parser to audio_resample_method="soxr" makes the input features match HF more closely without changing the global default for other models.

The MusicFlamingo generation test no longer skips missing fixtures because missing committed fixtures should be a test error, not a skip. The small warmup in test_musicflamingo.py is there because MI300/ROCm showed a first-inference exact-output drift during decoder kernel compilation; after a one-token warmup, the steady-state deterministic output matches.

cc @eustlb @hmellor Lmk what you guys think.

@mergify
Copy link
Copy Markdown
Contributor

mergify Bot commented May 21, 2026

Hi @AndreasKaratzas, the pre-commit checks have failed. Please run:

uv pip install pre-commit>=4.5.1
pre-commit install
pre-commit run --all-files

Then, commit the changes and push to your branch.

For future commits, pre-commit will run automatically on changed files before each commit.

Tip

Is mypy failing?
mypy is run differently in CI. If the failure is related to this check, please use the following command to run it locally:
# For mypy (substitute "3.10" with the failing version if needed)
pre-commit run --hook-stage manual mypy-3.10

@lashahub
Copy link
Copy Markdown
Contributor

Thanks for looping me in.

AF-Next should not be added as a separate vLLM architecture anymore. The current HF checkpoints use the existing MusicFlamingo architecture (model_type=musicflamingo, MusicFlamingoForConditionalGeneration).

I’ll wait for this PR to settle/land, then rebase #39011 on top and re-scope it to AF-Next checkpoint coverage through MusicFlamingo only. That should avoid duplicating the Flamingo processor/RoTE fixes here.

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

Labels

ci/build multi-modality Related to multi-modality (#4194) ready ONLY add when PR is ready to merge/full CI is needed rocm Related to AMD ROCm

Projects

Status: Todo

Development

Successfully merging this pull request may close these issues.

5 participants