Skip to content

[llm] Update metadata max_seq_len based on the max range of dynamic shapes #11611

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 1 commit into from
Jun 18, 2025

Conversation

larryliu0820
Copy link
Contributor

@larryliu0820 larryliu0820 commented Jun 12, 2025

With some testing it seems like we can't export with token dimension max being max_seq_len in dynamic shape, if we only export with tokens.

However if we export with both tokens and input_pos, we can set token dimension max value to be max_seq_len.

This diff fix 2 things:

  • Change dynamic shape based on different inputs
  • Change pte's metadata get_max_seq_len and get_max_context_len based on the value of token dimension max value in dynamic shape.

Copy link

pytorch-bot bot commented Jun 12, 2025

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/executorch/11611

Note: Links to docs will display an error until the docs builds have been completed.

✅ No Failures

As of commit db863ef with merge base a6d8440 (image):
💚 Looks good so far! There are no failures yet. 💚

This comment was automatically generated by Dr. CI and updates every 15 minutes.

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Jun 12, 2025
@facebook-github-bot
Copy link
Contributor

@larryliu0820 has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

@larryliu0820 larryliu0820 added the release notes: none Do not include this in the release notes label Jun 12, 2025
@larryliu0820 larryliu0820 force-pushed the larryliu0820-patch-9 branch from e48419f to b45e4c1 Compare June 12, 2025 20:29
@facebook-github-bot
Copy link
Contributor

@larryliu0820 has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

facebook-github-bot pushed a commit that referenced this pull request Jun 12, 2025
Summary:
See if any CI is broken by this.


Differential Revision: D76530379

Pulled By: larryliu0820
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D76530379

facebook-github-bot pushed a commit that referenced this pull request Jun 13, 2025
Summary:
With some testing it seems like we can't export with token dimension max being `max_seq_len` in dynamic shape, if we only export with `tokens`.

However if we export with both `tokens` and `input_pos`, we can set token dimension max value to be `max_seq_len`.

This diff fix 2 things: 

* Change dynamic shape based on different inputs
* Change pte's metadata `get_max_seq_len` and `get_max_context_len` based on the value of token dimension max value in dynamic shape.


Differential Revision: D76530379

Pulled By: larryliu0820
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D76530379

facebook-github-bot pushed a commit that referenced this pull request Jun 17, 2025
Summary:
With some testing it seems like we can't export with token dimension max being `max_seq_len` in dynamic shape, if we only export with `tokens`.

However if we export with both `tokens` and `input_pos`, we can set token dimension max value to be `max_seq_len`.

This diff fix 2 things: 

* Change dynamic shape based on different inputs
* Change pte's metadata `get_max_seq_len` and `get_max_context_len` based on the value of token dimension max value in dynamic shape.


Test Plan:
Imported from GitHub, without a `Test Plan:` line.

To repro the issue with max value being `max_seq_len` for `tokens` only argument, change the `_get_dynamic_shape()` and run:

```
buck run fbcode//executorch/examples/models/llama/fb:cria05 -- cria_0_5b -E 8,0 -d fp32 cria.pte
```

Differential Revision: D76530379

Pulled By: larryliu0820
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D76530379

@larryliu0820 larryliu0820 changed the title [llm] See what happens if we export with max_seq_len [llm] Update metadata max_seq_len based on the max range of dynamic shapes Jun 17, 2025
@facebook-github-bot
Copy link
Contributor

@larryliu0820 has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

facebook-github-bot pushed a commit that referenced this pull request Jun 17, 2025
…11611)

Summary:
With some testing it seems like we can't export with token dimension max being `max_seq_len` in dynamic shape, if we only export with `tokens`.

However if we export with both `tokens` and `input_pos`, we can set token dimension max value to be `max_seq_len`.

This diff fix 2 things: 

* Change dynamic shape based on different inputs
* Change pte's metadata `get_max_seq_len` and `get_max_context_len` based on the value of token dimension max value in dynamic shape.


Reviewed By: kimishpatel

Differential Revision: D76530379

Pulled By: larryliu0820
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D76530379

…11611)

Summary:
With some testing it seems like we can't export with token dimension max being `max_seq_len` in dynamic shape, if we only export with `tokens`.

However if we export with both `tokens` and `input_pos`, we can set token dimension max value to be `max_seq_len`.

This diff fix 2 things: 

* Change dynamic shape based on different inputs
* Change pte's metadata `get_max_seq_len` and `get_max_context_len` based on the value of token dimension max value in dynamic shape.


Reviewed By: kimishpatel

Differential Revision: D76530379

Pulled By: larryliu0820
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D76530379

@facebook-github-bot facebook-github-bot merged commit 57e0765 into main Jun 18, 2025
97 of 98 checks passed
@facebook-github-bot facebook-github-bot deleted the larryliu0820-patch-9 branch June 18, 2025 03:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. fb-exported release notes: none Do not include this in the release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants