Skip to content

Update warmup log messages and comments #284

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Jul 8, 2025
Merged

Update warmup log messages and comments #284

merged 2 commits into from
Jul 8, 2025

Conversation

tjohnson31415
Copy link
Collaborator

Description

This came out of a follow up to #270 to determine why an extra Prefill was necessary after using warmup_mode. I learned that the extra Prefill is required to deploy the compiled graph to the Spyre device. This PR does not change any functionality, but updates logging and documentation around warmup to make this clearer.

Signed-off-by: Travis Johnson <[email protected]>
Copy link

github-actions bot commented Jul 7, 2025

👋 Hi! Thank you for contributing to vLLM support on Spyre.
Just a reminder: Make sure that your code passes all the linting checks, otherwise your PR won't be able to be merged. To do so, first install the linting requirements, then run format.sh and commit the changes. This can be done with uv directly:

uv sync --frozen --group lint --active --inexact

Or this can be done with pip:

uv pip compile --group lint > requirements-lint.txt
pip install -r requirements-lint.txt
bash format.sh

Now you are good to go 🚀

@tjohnson31415
Copy link
Collaborator Author

I took the liberty of updating most of the warmup logging: using a [WARMUP] tag in the message, more consistent formating, and combining adjacent log messages. But let me know if the preference would be to limit the changes to just update the "additional prefill" CB step.

Copy link
Collaborator

@prashantgupta24 prashantgupta24 left a comment

Choose a reason for hiding this comment

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

LGTM! Couple nits

Copy link
Collaborator

@yannicks1 yannicks1 left a comment

Choose a reason for hiding this comment

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

lgtm, if possible address @prashantgupta24 nits..

@joerunde
Copy link
Collaborator

joerunde commented Jul 8, 2025

get in!

@joerunde joerunde merged commit 2488fb5 into main Jul 8, 2025
15 of 18 checks passed
@joerunde joerunde deleted the warmup-logs branch July 8, 2025 17:32
tjohnson31415 pushed a commit that referenced this pull request Jul 15, 2025
### [SB] fix order of warmup print

order changed in this this PR: #284

Signed-off-by: Yannick Schnider <[email protected]>
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