Skip to content

[AMD] [CI] Fix tests for v0.28.0 - #6830

Merged
hsliuustc0106 merged 10 commits into
vllm-project:mainfrom
tjtanaa:prepv0280
Aug 31, 2026
Merged

hsliuustc0106 merged 10 commits into
vllm-project:mainfrom
tjtanaa:prepv0280

Conversation

@tjtanaa

@tjtanaa tjtanaa commented Aug 30, 2026 •

Copy link
Copy Markdown
Member

PLEASE FILL IN THE PR DESCRIPTION HERE.

Purpose

This is to fix most of the tests after upgrading to v0.28.0. tests/diffusion/offloader/test_diffusion_layerwise_offload.py::test_layerwise_offload_diffusion_model[stabilityai/stable-audio-open-1.0] in Diffusion Model CPU offloading Test test group is quarantined for triaging.

Test Plan

vLLM Version:

vLLM-Omni Commit:

Test Result

Buildkite: https://buildkite.com/vllm/vllm-omni-amd-ci/builds/11222/list

BEFORE SUBMITTING: read CONTRIBUTING.md and run the precheck-pr skill with the code agent for a self-check against project conventions.
(anything written below this line will be removed by GitHub Actions)

Signed-off-by: tjtanaa <tunjian.tan@embeddedllm.com>
Signed-off-by: tjtanaa <tunjian.tan@embeddedllm.com>
…rch.float32 != torch.bfloat16.

Signed-off-by: tjtanaa <tunjian.tan@embeddedllm.com>
Signed-off-by: tjtanaa <tunjian.tan@embeddedllm.com>
Signed-off-by: tjtanaa <tunjian.tan@embeddedllm.com>
Signed-off-by: tjtanaa <tunjian.tan@embeddedllm.com>
@chatgpt-codex-connector

Copy link
Copy Markdown

Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits.
Credits must be used to enable repository wide code reviews.

@vllm-omni-review-bot

Copy link
Copy Markdown

This PR was classified as CI work.

CI owner: @yenuo26

@tjtanaa, please review your own changes and leave a short self-review comment describing what you checked. PRs without author self-review may not be assigned a reviewer.

Please take a look when you have a chance. If you would like an automated review, mention @vllm-omni-review-bot in a comment.

Signed-off-by: tjtanaa <tunjian.tan@embeddedllm.com>
assert max_diff < 0.01, f"Max difference {max_diff} exceeds threshold 0.01"
assert mean_diff < 0.001, f"Mean difference {mean_diff} exceeds threshold 0.001"
# Match the platform-specific comparison used by test_fa_vs_sdpa above.
if current_omni_platform.is_rocm():

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

ROCm cross-attention test tolerance fix

Failure

tests/diffusion/attention/test_flash_attn.py::test_cross_attn_key_padding_vs_sdpa[40]
failed on ROCm with:

AssertionError: Max difference 0.015625 exceeds threshold 0.01

Triage and root cause

The regression test compares BF16 output from two different implementations:
AITER's variable-length FlashAttention kernel and PyTorch SDPA. The test reaches
the intended cross-attention route: all 256 query rows are retained while the
key/value rows are unpadded to the per-batch valid lengths of 17 and 29.

The failure is numerical rather than a masking or routing defect. On ROCm, the
two kernels use different BF16 reduction implementations. A maximum absolute
difference of 0.015625 is a BF16-sized rounding difference and can exceed a
fixed 0.01 absolute bound for larger-magnitude output values. The older
test_fa_vs_sdpa test in the same file already accounts for this known AITER
versus ROCm SDPA behavior with rtol=1e-2 and atol=1.5e-2.

Fix

The cross-attention regression now uses the same ROCm-specific
torch.testing.assert_close comparison as the existing FA-versus-SDPA test:

torch.testing.assert_close(output_fa, output_sdpa, rtol=1e-2, atol=1.5e-2)

CUDA and XPU retain the original strict maximum- and mean-difference checks.
This keeps the relaxation limited to the backend with the known numerical
behavior and still checks every output element, rather than only increasing a
global maximum-difference threshold.

if os.path.isdir(candidate):
self.tokenizer = candidate
logger.info("Downloaded tokenizer from %s/%s", model_path, subfolder)
logger.info("Downloaded tokenizer from %s/%s", model_path, tokenizer_subfolder)

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

CosyVoice3 TTS startup fix

Problem

The following end-to-end tests reported that the server processes exited before becoming ready:

  • test_voice_clone_zh_001[cosyvoice3]
  • test_voice_clone_zh_002[cosyvoice3_async_chunk]

During CosyVoice3 startup, OmniEngineArgs.create_model_config() downloads the model's tokenizer subfolder and points self.tokenizer at the resulting local directory. After a successful download, the informational log statement referenced subfolder, a variable that is not defined in the remote-model branch.

This raised UnboundLocalError inside the tokenizer setup block and incorrectly sent a successful download through its exception handler, producing:

Failed to download tokenizer subfolder: cannot access local variable 'subfolder' where it is not associated with a value

Fix

The log statement now uses tokenizer_subfolder, which is the variable selected and used by the remote download branch:

- logger.info("Downloaded tokenizer from %s/%s", model_path, subfolder)
+ logger.info("Downloaded tokenizer from %s/%s", model_path, tokenizer_subfolder)

This is deliberately scoped to the incorrect variable reference. It does not change model selection, download behavior, tokenizer paths, or error handling.

Regression coverage

A unit test exercises the remote CosyVoice3 tokenizer path with a successful mocked snapshot download. It verifies that:

  • the downloaded CosyVoice-BlankEN directory is assigned to args.tokenizer;
  • model configuration creation completes normally; and
  • the tokenizer-download warning path is not entered.

This test would fail before the fix because evaluating the success log statement raised UnboundLocalError.

Verification

The relevant checks pass after the change:

  • test_voice_clone_zh_001[cosyvoice3]: passed
  • test_voice_clone_zh_002[cosyvoice3_async_chunk]: passed
  • tests/engine/test_arg_utils.py: 25 passed
  • git diff --check: passed

The server startup log now reports the resolved tokenizer location without the false failure warning:

Downloaded tokenizer from FunAudioLLM/Fun-CosyVoice3-0.5B-2512/CosyVoice-BlankEN



@pytest.fixture(autouse=True)
def restore_torch_default_dtype():

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

This is to fix error from https://buildkite.com/vllm/vllm-omni-amd-ci/builds/11218/list?jid=01a0538e-b7f2-4d6c-8e4d-89d2dda0e999&tab=output

snippet:

FAILED tests/diffusion/offloader/test_distributed_layerwise_backend.py::TestMmapValidation::test_validate_loaded_weights_called - assert None is not None
2026-08-30 17:27:49 UTC
 +  where None = HostWeightPlanResult(plan=None, fallback_reason="dtype mismatch for 'transformer.blocks.0.weight': checkpoint=F32, runtime=torch.bfloat16").plan
2026-08-30 17:27:49 UTC
FAILED tests/diffusion/quantization/test_svdquant_linear.py::test_prepare_weights_uses_existing_nvfp4_layout_contract - AssertionError: The values for attribute 'dtype' do not match: torch.float32 != torch.bfloat16.

@hsliuustc0106 hsliuustc0106 added the high priority high priority issue, needs to be done asap label Aug 30, 2026


@pytest.fixture(autouse=True)
def restore_torch_default_dtype():

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

P3 (informational, root cause): the dtype leak this guards against originates at tests/diffusion/models/ovis_image/test_ovis_image.py:266 — torch.set_default_dtype(torch.bfloat16) with no restore. The wholesale CPU lane runs pytest tests/diffusion in one process, so ovis_image (collected before offloader/ and quantization/) leaves every later test under a bf16 default, matching the F32-checkpoint vs bf16-runtime mismatches in the linked build. The fixture fully covers tests/diffusion; optionally also restore it in the ovis_image test itself so the source stops leaking if it ever runs outside this tree.

@hsliuustc0106 hsliuustc0106 added CI/CD codes related to changes to CI/CD bug Something isn't working labels Aug 30, 2026

@hsliuustc0106 hsliuustc0106 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

All four fixes verified at head: the arg_utils subfolder fix is a real bug (success-path UnboundLocalError swallowed into a false failure warning) with sound regression coverage; the ROCm assert_close mirrors the in-file test_fa_vs_sdpa pattern exactly; the yml deselect ID matches the parametrize and the quarantined step still runs the test NonBlocking; the conftest fixture covers the cross-test default-dtype leak. One P3 posted on the conftest naming ovis_image:266 as the leak source. LGTM.

@hsliuustc0106 hsliuustc0106 added the ready label to trigger buildkite CI label Aug 31, 2026

@hsliuustc0106 hsliuustc0106 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

lgtm

@hsliuustc0106
hsliuustc0106 merged commit ded8934 into vllm-project:main Aug 31, 2026
9 checks passed
JoseCarlosGarcia95 pushed a commit to valendra-tech/vllm-omni that referenced this pull request Sep 5, 2026
Signed-off-by: tjtanaa <tunjian.tan@embeddedllm.com>
khairulkabir1661 pushed a commit to khairulkabir1661/vllm-omni that referenced this pull request Sep 25, 2026
Signed-off-by: tjtanaa <tunjian.tan@embeddedllm.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working CI/CD codes related to changes to CI/CD high priority high priority issue, needs to be done asap ready label to trigger buildkite CI

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants