-
Notifications
You must be signed in to change notification settings - Fork 218
config file based modelopt config 1/N #657
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
base: main
Are you sure you want to change the base?
Conversation
|
Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually. Contributors can view more details about this message here. |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #657 +/- ##
==========================================
- Coverage 74.66% 74.52% -0.15%
==========================================
Files 183 183
Lines 18550 18467 -83
==========================================
- Hits 13851 13763 -88
- Misses 4699 4704 +5 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
d5cba37 to
9c326f8
Compare
1. start a new config system using yaml/yml files. The config system adopts Hydra style override, using the defaults tag, but we implement it by ourselves by simply merging the configs using OmegaConf 2. implement the quantization configs using the new config system, but not actually in use. 3. make sure the configs from the new config system match the exisiting configs 4. pipe through config_file based hf_ptq script 5. testted hf_ptq using both builtin and extenal config file Signed-off-by: Shengliang Xu <[email protected]>
and they block custom quantization config Signed-off-by: Shengliang Xu <[email protected]>
Signed-off-by: Shengliang Xu <[email protected]>
9c326f8 to
2650434
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do we keep this in modelopt/config/quantization/ instead of modelopt/torch/quantization/config?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1 on this. Since we already have modelopt/torch/quantization/config.py, can we move them to modelopt/torch/quantization/config_yamls/ or something?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because I want to make the builtin configs first class citizens. The configs are data, I want to put all built-in configs in one centralized place rather than scatter them around in code folders.
And I plan to convert all modelopt configs to be config file based.
The quantization configs are now under modelopt/config/quantization/, other configs will be placed in correponding sub-folder of modelopt/config.
The actual sub-dirs of each config type may change, but overall the idea of put all of them in a central place will be kept.
|
Overall design looks great to me. Can we make the yaml file flexible to add more methods such as auto_quantize or sparsity? the sparsity + quantization will be very important in future Cc @kaix-nv Should we have one more level to the yaml file based on the high level API: |
This is just 1/N of the changes. The use of the new config system is just limited to one use case just for illustration. More broader adaption will be in follow-up changes. |
|
The more I think about it, the more I feel it may be worth a design review of itself. I'll put together a more thorough design doc and share it. |
What does this PR do?
Type of change: ?
new feature Jira: [OMNIML-3212]
Overview: ?
start a new config system using yaml/yml files. The config system adopts Hydra style override, using the defaults tag, but we implement it by ourselves by simply merging the configs using OmegaConf
implement the quantization configs using the new config system, but not actually in use.
make sure the configs from the new config system match the exisiting configs
pipe through config_file based hf_ptq script
testted hf_ptq using both builtin and extenal config file
The config files are not actually in use in this PR, except the hf_ptq.py script accepts a quantization config name corresponding to a config filename
Testing