Skip to content

Conversation

mgoin
Copy link
Member

@mgoin mgoin commented Sep 17, 2025

Purpose

Test Plan

Test Result


Essential Elements of an Effective PR Description Checklist
  • The purpose of the PR, such as "Fix some issue (link existing issues this PR will resolve)".
  • The test plan, such as providing test command.
  • The test results, such as pasting the results comparison before and after, or e2e results
  • (Optional) The necessary documentation update, such as updating supported_models.md and examples for a new model.
  • (Optional) Release notes update. If your change is user facing, please update the release notes draft in the Google Doc.

Signed-off-by: mgoin <mgoin64@gmail.com>
Copy link
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 disables a failing GPT-OSS evaluation test for Blackwell GPUs in the CI pipeline by marking it as optional. While this is a reasonable temporary measure to unblock development, it introduces a risk of the test remaining disabled indefinitely. My review includes a suggestion to add a tracking issue to ensure the test is eventually fixed and re-enabled.

working_dir: "/vllm-workspace/"
gpu: b200
# optional: true
optional: true # disable while debugging
Copy link
Contributor

Choose a reason for hiding this comment

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

high

While disabling a failing test is a reasonable temporary measure to unblock the CI, there's a risk it might be forgotten and left disabled permanently. This would prevent catching regressions in the GPT-OSS evaluation on Blackwell GPUs. To mitigate this, it's highly recommended to create a tracking issue to fix and re-enable this test, and reference the issue number in a comment here.

  optional: true # TODO(<issue-link>): Re-enable this test after fixing the failures.

@mergify mergify bot added ci/build gpt-oss Related to GPT-OSS models labels Sep 17, 2025
@vllm-bot vllm-bot merged commit 9f882d8 into vllm-project:main Sep 17, 2025
9 of 11 checks passed
@github-project-automation github-project-automation bot moved this from To Triage to Done in gpt-oss Issues & Enhancements Sep 17, 2025
debroy-rh pushed a commit to debroy-rh/vllm that referenced this pull request Sep 19, 2025
FeiDaLI pushed a commit to FeiDaLI/vllm that referenced this pull request Sep 25, 2025
charlifu pushed a commit to ROCm/vllm that referenced this pull request Sep 25, 2025
Signed-off-by: mgoin <mgoin64@gmail.com>
Signed-off-by: charlifu <charlifu@amd.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ci/build gpt-oss Related to GPT-OSS models
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

2 participants