Skip to content

added fixes for handling multiple shape warmup #13

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

JRosenkranz
Copy link
Contributor

This PR is to address issues with warming up multiple shapes on the AIU. This PR introduces the use of a prepare_model_inputs_hook which will mark certain dimensions as static/dynamic prior to forward pass. This relies on the following PR foundation-model-stack/foundation-model-stack#388

…dded tests for multiple shape warmup

Signed-off-by: Joshua Rosenkranz <jmrosenk@us.ibm.com>
@JRosenkranz JRosenkranz requested a review from ani300 April 1, 2025 14:38
@JRosenkranz JRosenkranz self-assigned this Apr 1, 2025

for layer in kwargs["past_key_value_states"]:
for tensor in layer:
torch._dynamo.mark_static(tensor, 0)
Copy link
Contributor

Choose a reason for hiding this comment

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

we could move the mark kv cache sequence dimension as dynamic code here as well

Comment on lines 27 to 28
torch._dynamo.mark_dynamic(kwargs["mask"], 1)
torch._dynamo.mark_dynamic(kwargs["mask"], 2)
Copy link
Contributor

Choose a reason for hiding this comment

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

we probably only need to mark the dim 2 as dynamic here

Comment on lines +17 to +23
torch._dynamo.mark_static(input_ids, 0)
torch._dynamo.mark_static(input_ids, 1)
torch._dynamo.mark_static(kwargs["mask"], 0)
torch._dynamo.mark_static(kwargs["mask"], 1)
torch._dynamo.mark_static(kwargs["mask"], 2)
torch._dynamo.mark_static(kwargs["position_ids"], 0)
torch._dynamo.mark_static(kwargs["position_ids"], 1)
Copy link
Contributor

Choose a reason for hiding this comment

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

do we need to mark all the sequence dimensions as static or is just the batch dimensions enough?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It probably is enough, however I marked everything as static to ensure we get a static prefill -- I believe symbolic ints can cause changes in the graph in prefill that we may not want to introduce.

Signed-off-by: Joshua Rosenkranz <jmrosenk@us.ibm.com>
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.

2 participants