Conversation
Differential Revision: [D92304723](https://our.internmc.facebook.com/intern/diff/D92304723/) [ghstack-poisoned]
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/executorch/17229
Note: Links to docs will display an error until the docs builds have been completed. ❌ 3 New Failures, 2 Unrelated FailuresAs of commit 9876e7d with merge base eac0673 ( NEW FAILURES - The following jobs have failed:
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. |
This PR needs a
|
Differential Revision: [D92304723](https://our.internmc.facebook.com/intern/diff/D92304723/) [ghstack-poisoned]
There was a problem hiding this comment.
Pull request overview
This PR introduces a LoraConfig dataclass to encapsulate LoRA (Low-Rank Adaptation) adapter configuration, replacing the previous separate adapter_checkpoint and adapter_config fields in BaseConfig. This refactoring improves code organization and provides a cleaner API for LoRA configuration.
Changes:
- Added
LoraConfigdataclass with validation and automatic config parsing from JSON - Refactored
BaseConfigto useLoraConfiginstead of separate adapter fields - Updated
from_argsmethod to createLoraConfiginstances from CLI arguments - Simplified adapter loading logic in
Llama2Modelby using the newLoraConfigobject
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| extension/llm/export/config/llm_config.py | Introduces LoraConfig dataclass with validation, updates BaseConfig to use it, and modifies from_args to create LoraConfig from CLI arguments |
| examples/models/llama/model.py | Refactors adapter loading to use LoraConfig object, simplifying the logic by removing redundant validation and JSON parsing |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| import json | ||
| import os | ||
|
|
There was a problem hiding this comment.
Remove these duplicate imports. The modules json and os are already imported at the top of the file (lines 21-22), so importing them again inside this method is unnecessary.
| import json | |
| import os |
| """ | ||
|
|
||
| import argparse | ||
| import json |
There was a problem hiding this comment.
Import of 'json' is not used.
| import json |
|
|
||
| import argparse | ||
| import json | ||
| import os |
There was a problem hiding this comment.
Import of 'os' is not used.
| import os |
Stack from ghstack (oldest at bottom):
Differential Revision: D92304723