[Tests] Weekly lm eval tests - #3022
Conversation
…stale configs - Change w4a4_nvfp4 and w4a16_actorder_weight cadence from weekly to nightly - Add FP8 block quantization config with nightly cadence (calibrated thresholds) - Disable fp8_static_per_tensor and w4a16_actorder_group by moving to disabled_configs/ Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: Kyle Sayers <kylesayrs@gmail.com>
Signed-off-by: Kyle Sayers <kylesayrs@gmail.com>
|
👋 Hi! Thank you for contributing to llm-compressor. Please add the ready label when the PR is ready for review. Note: This is required to complete the testing suite, please only add the label once the PR is code complete and local testing has been performed. |
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces new quantization recipes and evaluation configurations, updates several test cadences from weekly to nightly, and adds support for skipping expensive base model evaluations by providing pre-defined base_metrics in test_lmeval.py. The review feedback suggests improving type safety in test_lmeval.py by using Union[dict, None] for the optional base_metrics field, and explicitly checking is not None to prevent empty dictionaries from triggering the base model evaluation. Additionally, there are minor typos ('cpabilities') to correct in the comments of several configuration files.
| # Optional base model metrics to skip base model evaluation | ||
| base_metrics: dict = None |
There was a problem hiding this comment.
Using dict = None as a type annotation for an optional field can cause type checking or Pydantic validation issues. It is more robust and standard to use Union[dict, None] = None (or Optional[dict] = None) to explicitly declare that the field can be None.
| # Optional base model metrics to skip base model evaluation | |
| base_metrics: dict = None | |
| # Optional base model metrics to skip base model evaluation | |
| base_metrics: Union[dict, None] = None |
| torch.accelerator.synchronize() | ||
| # Give GPU time to fully release memory | ||
| time.sleep(2) | ||
| if self.config.lmeval.base_metrics: |
There was a problem hiding this comment.
Checking if self.config.lmeval.base_metrics: will evaluate to False if base_metrics is an empty dictionary {}. This would cause the code to silently fall back to the else block and run the expensive base model evaluation, which defeats the purpose of skipping it. Using is not None ensures that any explicitly provided dictionary (even if empty) is handled correctly and fails fast during validation.
| if self.config.lmeval.base_metrics: | |
| if self.config.lmeval.base_metrics is not None: |
| # Note: recovery will be dependent on | ||
| # the hardware cpabilities. For blackwell | ||
| # machines, we expect lower results as | ||
| # activation quantization is also enabled. |
There was a problem hiding this comment.
Fix typo 'cpabilities' to 'capabilities' in the comment.
# Note: recovery will be dependent on
# the hardware capabilities. For blackwell
# machines, we expect lower results as
# activation quantization is also enabled.| # Note: recovery will be dependent on | ||
| # the hardware cpabilities. For blackwell | ||
| # machines, we expect lower results as | ||
| # activation quantization is also enabled. |
There was a problem hiding this comment.
Fix typo 'cpabilities' to 'capabilities' in the comment.
# Note: recovery will be dependent on
# the hardware capabilities. For blackwell
# machines, we expect lower results as
# activation quantization is also enabled.| # Note: recovery will be dependent on | ||
| # the hardware cpabilities. For blackwell | ||
| # machines, we expect lower results as | ||
| # activation quantization is also enabled. |
There was a problem hiding this comment.
Fix typo 'cpabilities' to 'capabilities' in the comment.
# Note: recovery will be dependent on
# the hardware capabilities. For blackwell
# machines, we expect lower results as
# activation quantization is also enabled.
Merge Protections🔴 1 of 1 protections blocking · waiting on 👀 reviews
🔴 Require one maintainer reviewWaiting for any of
This rule is failing.All PRs must have at least one approving review from a maintainer before merging.
|
1 participant
No description provided.