[error message improvement] include num_calibration_samples/max_seq_length in oom resolution message - #3020
[error message improvement] include num_calibration_samples/max_seq_length in oom resolution message#3020brian-dellabetta wants to merge 2 commits into
Conversation
Signed-off-by: Brian Dellabetta <bdellabe@redhat.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 updates the OutOfMemoryError message in the sequential pipeline helper to suggest reducing the calibration dataset size (via num_calibration_samples or max_seq_length) as an alternative solution. The reviewer suggested changing 'data set' to 'dataset' in the error message for consistency with the rest of the codebase.
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com> Signed-off-by: Brian Dellabetta <brian-dellabetta@users.noreply.github.com>
|
This pull request has merge conflicts that must be resolved before it can be |
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.
|
When `max_seq_length` is unset and the tokenized calibration dataset contains samples longer than SEQ_LEN_ERROR_THRESHOLD (2048) tokens, format_calibration_data() now raises a ValueError instead of warning. Calibrating with long untruncated samples runs out of GPU memory with the OOM raised from attention or attention mask expansion, which is easily mistaken for the model not fitting on the device, and a warning for it is buried in noisy logs while an error surfaces at the failure point (per review). The message reports how many samples exceed the threshold and the longest length, and gives both remedies: set `max_seq_length` to truncate, or set it to at least the longest length to calibrate on full-length samples intentionally. Sample lengths are measured on the arrow column to avoid materializing the tokenized dataset in Python memory. Also fold in vllm-project#3020's guidance so it can close in favor of this PR: the sequential pipeline's OOM message now also suggests reducing `num_calibration_samples` or `max_seq_length`. Fixes vllm-project#3011 Co-authored-by: Brian Dellabetta <bdellabe@redhat.com> Signed-off-by: Rishabh Sinha <rsinha17@terpmail.umd.edu>
When `max_seq_length` is unset and the tokenized calibration dataset contains samples longer than SEQ_LEN_ERROR_THRESHOLD (2048) tokens, format_calibration_data() now raises a ValueError instead of warning. Calibrating with long untruncated samples runs out of GPU memory with the OOM raised from attention or attention mask expansion, which is easily mistaken for the model not fitting on the device, and a warning for it is buried in noisy logs while an error surfaces at the failure point (per review). The message reports how many samples exceed the threshold and the longest length, and gives both remedies: set `max_seq_length` to truncate, or set it to at least the longest length to calibrate on full-length samples intentionally. Sample lengths are measured on the arrow column to avoid materializing the tokenized dataset in Python memory. Also fold in vllm-project#3020's guidance so it can close in favor of this PR: the sequential pipeline's OOM message now also suggests reducing `num_calibration_samples` or `max_seq_length`. Fixes vllm-project#3011 Co-authored-by: Brian Dellabetta <bdellabe@redhat.com> Signed-off-by: Rishabh Sinha <rsinha17@terpmail.umd.edu>
2 participants
Fixes #3011
SUMMARY:
Update the OOM error message to include calibration dataset considerations.
TEST PLAN:
n/a