Fix eigenvalue parsing for compression-only quantize configs - #8057
Conversation
Sorry, something went wrong.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2e42aef6fb
ℹ️ About Codex in GitHubCodex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
Sorry, something went wrong.
Signed-off-by: Sowndappan S <147894621+sowndappan5@users.noreply.github.com>
Signed-off-by: Sowndappan S <147894621+sowndappan5@users.noreply.github.com>
|
Hi @sowndappan5, The parser change looks correct to me. But it seems to have an issue in the new test. The new test_quantized_eigenvalue_config_parses test currently uses get_test_path('model/BingBertSquad/...'). get_test_path() resolves relative to tests/unit, so this points at tests/unit/model/.... The fixture is actually under tests/model/BingBertSquad. Changing the path to get_test_path('../model/BingBertSquad/deepspeed_bsz24_fp16_eigenvalue_quantize_config.json') |
Sorry, something went wrong.
|
Thanks for the review and for pointing this out. I'll check the test path on my end and verify the behavior you described. If that's the issue, I'll update the test accordingly and push a fix. |
Sorry, something went wrong.
…arses Signed-off-by: sowndappan5 <sowndappan610@gmail.com>
|
@tohtana I have updated the test path to ../model/BingBertSquad/... and pushed the fix. All local pre-commit checks are passing now. Please take another look! |
Sorry, something went wrong.
tohtana
left a comment
There was a problem hiding this comment.
Thank you, @sowndappan5! Let's merge it now.
Sorry, something went wrong.
Description
This PR fixes eigenvalue configuration parsing in deepspeed/runtime/config.py for quantization-enabled configs.
Previously, get_eigenvalue_config() assumed the legacy top-level quantize_training section always existed after get_quantize_enabled() returned true. That caused a KeyError for supported compression configs that enable weight quantization through compression_training without also defining quantize_training.
This change makes the parser fall back to the default eigenvalue settings when the legacy section is absent, while preserving the existing behavior when quantize_training is present.
Testing
Added a regression test covering a compression_training weight-quantization config without a legacy quantize_training section.
Validated locally with the repository's focused pre-commit checks.