Skip to content

Make multimethod generic in llm_config#18213

Merged
meta-codesync[bot] merged 3 commits intogh/lucylq/140/basefrom
gh/lucylq/140/head
Mar 27, 2026
Merged

Make multimethod generic in llm_config#18213
meta-codesync[bot] merged 3 commits intogh/lucylq/140/basefrom
gh/lucylq/140/head

Conversation

@lucylq
Copy link
Copy Markdown
Contributor

@lucylq lucylq commented Mar 16, 2026

Stack from ghstack (oldest at bottom):

Differential Revision: D96822523

@pytorch-bot
Copy link
Copy Markdown

pytorch-bot Bot commented Mar 16, 2026

🔗 Helpful Links

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

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

✅ You can merge normally! (2 Unrelated Failures)

As of commit f42c141 with merge base 28b4813 (image):

BROKEN TRUNK - The following jobs failed but were present on the merge base:

👉 Rebase onto the `viable/strict` branch to avoid these failures

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

lucylq pushed a commit that referenced this pull request Mar 16, 2026
Differential Revision: [D96822523](https://our.internmc.facebook.com/intern/diff/D96822523/)

ghstack-source-id: 352958349
Pull Request resolved: #18213
@meta-cla meta-cla 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 Mar 16, 2026
@github-actions
Copy link
Copy Markdown

This PR needs a release notes: label

If your change should be included in the release notes (i.e. would users of this library care about this change?), please use a label starting with release notes:. This helps us keep track and include your important work in the next release notes.

To add a label, you can comment to pytorchbot, for example
@pytorchbot label "release notes: none"

For more information, see
https://github.com/pytorch/pytorch/wiki/PyTorch-AutoLabel-Bot#why-categorize-for-release-notes-and-how-does-it-work.

@lucylq lucylq requested a review from JacobSzwejbka March 16, 2026 23:13
Copy link
Copy Markdown
Contributor

@kimishpatel kimishpatel left a comment

Choose a reason for hiding this comment

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

Review automatically exported from Phabricator review in Meta.

lucylq pushed a commit that referenced this pull request Mar 26, 2026
Pull Request resolved: #18213

MultimethodLoraConfig --> MultimethodConfig

Introduce MethodConfig for per-method configs (currently just lora). Default to the base llm_config params for everything else.

Program-wide configs (such as share_mutable_buffers) stay under MultimethodConfig.


ghstack-source-id: 358171273
@exported-using-ghexport

Differential Revision: [D96822523](https://our.internmc.facebook.com/intern/diff/D96822523/)
@lucylq lucylq requested a review from Copilot March 26, 2026 16:06
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR generalizes the LLM export “multimethod” configuration by renaming the previous multimethod_lora configuration to a more generic multimethod and switching from a dict-based schema to a list of per-method config objects, then updating the example YAML and llama export path accordingly.

Changes:

  • Replace MultimethodLoraConfig with MultimethodConfig + new MethodConfig (list-based schema) in LlmConfig.
  • Update Qwen3 multimethod example YAML to the new schema.
  • Update llama export/validation logic to consume llm_config.multimethod and iterate through method configs.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File Description
extension/llm/export/config/llm_config.py Introduces MethodConfig and replaces multimethod_lora with a generic multimethod list-based configuration.
examples/models/qwen3/config/qwen3_multimethod.yaml Updates the Qwen3 multimethod example to use the new multimethod.methods list format.
examples/models/llama/export_llama_lib.py Updates validation and multimethod export logic to use the new config shape and naming.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines 317 to 321
"""Configuration for exporting multiple methods to a single .pte file.

Maps method names to optional LoRA configurations. A None value means
the method uses base model weights.

Copy link

Copilot AI Mar 26, 2026

Choose a reason for hiding this comment

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

The MultimethodConfig docstring still says it "Maps method names" even though methods is now a List[MethodConfig]. This is misleading for config authors; please update the docstring wording to reflect the list-based schema (and, if relevant, that each list element’s lora_config=None means base weights).

Copilot uses AI. Check for mistakes.
Comment on lines 1309 to +1312

# Get the exported program
exported_program = builder._export(builder.pre_autograd_graph_module)
method_to_program[method_name] = exported_program
method_to_program[method.method_name] = exported_program
Copy link

Copilot AI Mar 26, 2026

Choose a reason for hiding this comment

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

With multimethod.methods now being a list, duplicate method_name entries are possible. The current code will silently overwrite earlier entries in method_to_program[...], but num_methods/logging still counts duplicates, which can lead to confusing or incorrect exports. Please validate that all method.method_name values are unique (and non-empty) before populating method_to_program, and raise a clear error if duplicates are found.

Copilot uses AI. Check for mistakes.
lucylq pushed a commit that referenced this pull request Mar 26, 2026
Pull Request resolved: #18213

MultimethodLoraConfig --> MultimethodConfig

Introduce MethodConfig for per-method configs (currently just lora). Default to the base llm_config params for everything else.

Program-wide configs (such as share_mutable_buffers) stay under MultimethodConfig.


ghstack-source-id: 358256777
@exported-using-ghexport

Differential Revision: [D96822523](https://our.internmc.facebook.com/intern/diff/D96822523/)
@meta-codesync meta-codesync Bot merged commit df879a8 into gh/lucylq/140/base Mar 27, 2026
157 of 162 checks passed
@meta-codesync meta-codesync Bot deleted the gh/lucylq/140/head branch March 27, 2026 13:34
@meta-codesync meta-codesync Bot temporarily deployed to cherry-pick-bot March 27, 2026 13:34 Inactive
lucylq pushed a commit that referenced this pull request Mar 27, 2026
This PR was created by the merge bot to help merge the original PR into
the main branch.
ghstack PR number: #18213 by
@lucylq
^ Please use this as the source of truth for the PR details, comments,
and reviews
ghstack PR base:
https://github.com/pytorch/executorch/tree/gh/lucylq/140/base
ghstack PR head:
https://github.com/pytorch/executorch/tree/gh/lucylq/140/head
Merge bot PR base: https://github.com/pytorch/executorch/tree/main
Merge bot PR head:
https://github.com/pytorch/executorch/tree/gh/lucylq/140/orig
Differential Revision:
[D96822523](https://our.internmc.facebook.com/intern/diff/D96822523/)
@diff-train-skip-merge

Co-authored-by: Github Executorch <github_executorch@arm.com>
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 meta-exported

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants