Skip to content

Conversation

@arpitkh101
Copy link
Contributor

@arpitkh101 arpitkh101 commented Nov 28, 2025

Purpose

Implements the PassConfig flag renaming from issue #27995.

Changes:

  • Renamed all PassConfig flags to be more descriptive (e.g., enable_fusion → fuse_norm_quant + fuse_act_quant)
  • Added deprecation warnings for old flag names
  • Old flags still work for backward compatibility
  • Updated all internal code and tests to use new names

@github-actions
Copy link

👋 Hi! Thank you for contributing to the vLLM project.

💬 Join our developer Slack at https://slack.vllm.ai to discuss your PR in #pr-reviews, coordinate on features in #feat- channels, or join special interest groups in #sig- channels.

Just a reminder: PRs would not trigger full CI run by default. Instead, it would only run fastcheck CI which starts running only a small and essential subset of CI tests to quickly catch errors.

You ask your reviewers to trigger select CI tests on top of fastcheck CI.

Once the PR is approved and ready to go, your PR reviewer(s) can run CI to test the changes comprehensively before merging.

To run CI, PR reviewers can either: Add ready label to the PR or enable auto-merge.

If you have any questions, please reach out to us on Slack at https://slack.vllm.ai.

🚀

@mergify mergify bot added the v1 label Nov 28, 2025
@mergify
Copy link

mergify bot commented Nov 28, 2025

This pull request has merge conflicts that must be resolved before it can be
merged. Please rebase the PR, @arpitkh101.

https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/working-with-forks/syncing-a-fork

@mergify mergify bot added the needs-rebase label Nov 28, 2025
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 refactors PassConfig flags to be more descriptive, which is a great improvement for clarity. The changes are mostly mechanical renames across the test suite and configuration files. However, I've found a critical issue in the backward compatibility logic for the deprecated flags in vllm/config/compilation.py. The current implementation incorrectly handles cases where old flags are set to False, which would unintentionally enable features. I've provided a suggestion to fix this to ensure correct behavior.

Copy link

@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.

ℹ️ 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".

Signed-off-by: arpitkh101 <[email protected]>
@arpitkh101 arpitkh101 force-pushed the rename-passconfig-flags branch from 15411fa to 0c4eb98 Compare November 29, 2025 04:22
@mergify mergify bot removed the needs-rebase label Nov 29, 2025
@arpitkh101 arpitkh101 force-pushed the rename-passconfig-flags branch from ece81a1 to 289336c Compare November 29, 2025 05:43
Copy link
Collaborator

@ProExpertProg ProExpertProg left a comment

Choose a reason for hiding this comment

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

A few minor notes, looks ready otherwise! Could you also add a test that checks that specifying the old fields results in new fields set correctly and a warning (see #29557 for example)

@ProExpertProg ProExpertProg added this to the v0.12.0 milestone Dec 2, 2025
@ProExpertProg
Copy link
Collaborator

@arpitkh101 will you be able to finish this today so we can include it in the 0.12.0 release?

@hmellor
Copy link
Member

hmellor commented Dec 2, 2025

@arpitkh101 will you be able to finish this today so we can include it in the 0.12.0 release?

If not, we should bump the removal version. Deprecation messges must be present for a whole minor version.

@arpitkh101
Copy link
Contributor Author

I think I'll be able to push these changes today.

@ProExpertProg
Copy link
Collaborator

I think I'll be able to push these changes today.

@arpitkh101 please keep us posted! Someone else can pick up and help if you're not able to finish today, would really like to get this in for this release!

Signed-off-by: arpitkh101 <[email protected]>
@ProExpertProg ProExpertProg enabled auto-merge (squash) December 2, 2025 18:19
@github-actions github-actions bot added the ready ONLY add when PR is ready to merge/full CI is needed label Dec 2, 2025
auto-merge was automatically disabled December 2, 2025 21:19

Head branch was pushed to by a user without write access

@ProExpertProg ProExpertProg enabled auto-merge (squash) December 2, 2025 22:41
@ProExpertProg ProExpertProg merged commit d7284a2 into vllm-project:main Dec 3, 2025
53 checks passed
khluu pushed a commit that referenced this pull request Dec 3, 2025
Signed-off-by: arpitkh101 <[email protected]>
Co-authored-by: Luka Govedič <[email protected]>
(cherry picked from commit d7284a2)
Copy link
Member

@hmellor hmellor left a comment

Choose a reason for hiding this comment

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

Couple of nits, not necessary for the release but would be nice to clean up asap.

@ProExpertProg ProExpertProg linked an issue Dec 3, 2025 that may be closed by this pull request
1 task
charlotte12l pushed a commit to charlotte12l/vllm that referenced this pull request Dec 5, 2025
…oject#29646)

Signed-off-by: arpitkh101 <[email protected]>
Co-authored-by: Luka Govedič <[email protected]>
Signed-off-by: Xingyu Liu <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready ONLY add when PR is ready to merge/full CI is needed v1

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[RFC]: Make PassConfig flags less verbose

3 participants