Skip to content

Conversation

yannicks1
Copy link
Collaborator

WIP

Copy link

github-actions bot commented Jun 3, 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 🚀

@wallashss
Copy link
Collaborator

FYI: I did something similar in this other draft PR. My intention is try have common between static batching and continuous batching and in after that have a structure that is more similar to vLLM upstream.

@yannicks1
Copy link
Collaborator Author

thanks for letting me know (I usually do not check draft PRs:)
What do you think about this change? I saw that the order is not the same for SB and CB and just changed it, test seem to pass...

torch._dynamo.mark_static(model_input.slot_mapping, 1) # always 1
torch._dynamo.mark_static(model_input.input_positions,
1) # always 1
self._mark_input_tensors(model_input)
Copy link
Collaborator

Choose a reason for hiding this comment

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

do we really need this function to be using self? It only interacts with the model_input param

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

you are totally right, I copied it from the static batching code, where it also uses self which can be removed. FYI: @wallashss

@yannicks1
Copy link
Collaborator Author

If @wallashss can confirm above change I will close this PR and he can address all in his?

@wallashss
Copy link
Collaborator

If @wallashss can confirm above change I will close this PR and he can address all in his?

I'm back to work in this refactoring, so sure I can address those.

@yannicks1
Copy link
Collaborator Author

Thanks Wallas, closing this one now.

@yannicks1 yannicks1 closed this Jun 12, 2025
@yannicks1 yannicks1 deleted the ysc-refactor-mark-tensors branch June 24, 2025 10:39
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.

3 participants