Skip to content

Conversation

adobrzyn
Copy link
Collaborator

@adobrzyn adobrzyn commented Oct 13, 2025

No flag -> default
Query -> query value
Seq -> seq value + "will be depricated"warning
Query & seq -> query value

Signed-off-by: Agata Dobrzyniewicz <[email protected]>
Copy link

🚧 CI Blocked

The main CI workflow was not started for the following reason:

Your branch is behind the base branch. Please merge or rebase to get the latest changes.

@PatrykWo PatrykWo self-requested a review October 13, 2025 13:32
Copy link

@jkaniecki jkaniecki left a comment

Choose a reason for hiding this comment

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

Please look at the proposed changes, also change 'seq' to 'query' in line 88 vllm_gaudi/extension/bucketing/linear.py

for p, e, d in zip(params, env_vars, default_values):
val = os.environ.get(e)

if val is None and dim == 'query':

Choose a reason for hiding this comment

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

As far as I know we don't have VLLM_DECODE_SEQ_BUCKET_{p} - is there a need for making this code handle such a case ? This code looks like it would need to

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

No, we don't set this dim - nor query nor seq, for decode

Choose a reason for hiding this comment

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

Ok, so shouldn't we put also "and phase == 'prompt' and then set
fallback_env = f'VLLM_PROMPT_SEQ_BUCKET_{p}'.upper() in 102 line ?

Copy link
Collaborator

@PatrykWo PatrykWo left a comment

Choose a reason for hiding this comment

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

LGTM, can we port that to 0.11.0?

Signed-off-by: Agata Dobrzyniewicz <[email protected]>
Copy link

🚧 CI Blocked

The main CI workflow was not started for the following reason:

Your branch is behind the base branch. Please merge or rebase to get the latest changes.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants