Skip to content

Avoid CUDA indexing failures in iMatrix grid search updates - #2997

Open
yasu-oh wants to merge 2 commits into
vllm-project:mainfrom
yasu-oh:main
Open

Avoid CUDA indexing failures in iMatrix grid search updates#2997
yasu-oh wants to merge 2 commits into
vllm-project:mainfrom
yasu-oh:main

Conversation

@yasu-oh

@yasu-oh yasu-oh commented Aug 3, 2026

Copy link
Copy Markdown

Summary

Replace boolean-indexed in-place updates in the iMatrix grid search with shape-validated torch.where selections.

The change:

  • validates that all update tensors and the boolean mask match best_error.shape
  • avoids the CUDA advanced-indexing path used by the masked assignments
  • preserves the existing no-improvement and patience behavior

Problem

The current update path can fail during CUDA quantization runs:

python
best_error[improved] = err[improved]
best_min[improved] = shrink_min[improved]
best_max[improved] = shrink_max[improved]

The following failures were observed with GPTQ, NVFP4, and the imatrix_mse observer:

  • Gemma 4 E2B: inconsistent selection sizes when the same boolean mask was used on both sides of an assignment
  • Gemma 4 E4B: CUDA IndexKernel.cu out-of-bounds device assertions

The exact lower-level cause has not been confirmed, so this change does not assume a specific PyTorch defect. The added shape validation did not trigger during successful patched GPU runs.

Validation

The development environment used to prepare this change is CPU-only.

CPU validation:

  • pytest -q tests/llmcompressor/observers/test_imatrix.py
    • 34 passed
  • ruff check
    • passed
  • ruff format --check
    • passed
  • git diff --check
    • passed
  • added a CPU regression test for explicit grid-search shape validation

GPU validation was performed separately using GPTQModifier with NVFP4 imatrix_mse, 1,024 calibration samples, max_seq_length=8192, FP8 static KV-cache calibration, and the basic calibration pipeline for Gemma 4 E models:

  • Gemma 4 E2B failed repeatedly before the change and completed on the first run after the change
  • Gemma 4 E4B hit CUDA IndexKernel.cu assertions before the change and completed on the first run after the change
  • the shape validation did not trigger in either successful run
  • quantization and compressed-model saving completed successfully

The same patched environment was used to produce the Gemma 4 QAT NVFP4 collection covering 31B, 26B-A4B, 12B, E4B, and E2B:

https://huggingface.co/collections/yasu-oh/gemma-4-qat-nvfp4

Trade-off

torch.where materializes full output tensors, so it may use slightly more temporary memory and memory bandwidth than masked in-place assignment.

Replace boolean-indexed in-place updates in the iMatrix grid search with shape-validated torch.where selections.

This avoids the CUDA advanced-indexing path that produced inconsistent same-mask selection sizes or IndexKernel out-of-bounds assertions in GPU integration runs, while preserving the intended element-wise update semantics.

Signed-off-by: yasu-oh <84763339+yasu-oh@users.noreply.github.com>
@coderabbitai

coderabbitai Bot commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: cdaca776-2792-4b08-91eb-bbf49bd20f18

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@mergify mergify Bot added the two-reviews When a PR requires two reviews label Aug 3, 2026
@mergify

mergify Bot commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

Merge Protections

🔴 2 of 2 protections blocking · waiting on 👀 reviews

Protection Waiting on
🔴 Require one maintainer review 👀 reviews
🔴 Require two reviews 👀 reviews

🔴 Require one maintainer review

Waiting for any of

  • approved-reviews-by=HDCharles
  • approved-reviews-by=brian-dellabetta
  • approved-reviews-by=dsikka
  • approved-reviews-by=kylesayrs
  • approved-reviews-by=yiliu30
This rule is failing.

All PRs must have at least one approving review from a maintainer before merging.

  • any of:
    • approved-reviews-by=HDCharles
    • approved-reviews-by=brian-dellabetta
    • approved-reviews-by=dsikka
    • approved-reviews-by=kylesayrs
    • approved-reviews-by=yiliu30
  • #changes-requested-reviews-by = 0

🔴 Require two reviews

Waiting for

  • #approved-reviews-by >= 2
This rule is failing.

PRs labelled "two-reviews" must have at least two approving reviews before merging.

  • #approved-reviews-by >= 2
  • #changes-requested-reviews-by = 0

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces shape validation checks and updates the tensor assignment logic using torch.where within the _grid_search function of the iMatrix observer, accompanied by a new unit test to verify that shape mismatches raise a RuntimeError. The feedback suggests optimizing this validation by checking only the shape of err on the first iteration, thereby avoiding unnecessary Python overhead in a hot loop.

Comment thread src/llmcompressor/observers/imatrix.py Outdated
Check the error shape only on the first grid-search iteration before computing the improvement mask. The remaining update tensor shapes are fixed by construction, so this avoids repeated dictionary allocation and redundant checks in the hot loop.

Signed-off-by: yasu-oh <84763339+yasu-oh@users.noreply.github.com>
@github-actions

github-actions Bot commented Aug 3, 2026

Copy link
Copy Markdown

👋 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.

@brian-dellabetta brian-dellabetta left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @yasu-oh , the changes look fine to me, but could you provide a minimal reproducible example script that triggers this "CUDA IndexKernel.cu out-of-bounds device assertion" error you are reporting? I've not seen it before

@yasu-oh

yasu-oh commented Aug 5, 2026

Copy link
Copy Markdown
Author

Thanks. I was able to reproduce the E4B failure again, although it appears to be intermittent rather than deterministic.

On the same system and configuration, it reproduced once in six fresh-process runs. In the failed run, the CUDA IndexKernel.cu out-of-bounds assertion was followed by a traceback that surfaced at:

best_max[improved] = shrink_max[improved]

in llmcompressor/observers/imatrix.py, ending with a CUDA device-side assert.

Because CUDA operations may be asynchronous, I am not yet claiming that this exact assignment is definitively the original source of the failure. I am currently preparing a standalone reproduction script and collecting the relevant logs, including runs with CUDA_LAUNCH_BLOCKING=1, and will share them once they are ready.

@yasu-oh

yasu-oh commented Aug 6, 2026

Copy link
Copy Markdown
Author

A quick update on the E4B case: although I observed the IndexKernel assertion once, I have not been able to reproduce it again after more than 20 additional fresh-process runs under the same conditions.

Given the lack of reliable reproducibility, please disregard the E4B-specific report for now. I do not want it to distract from the consistently reproducible E2B failure. I will revisit the E4B case separately if I can reproduce it reliably and collect stronger evidence.

@brian-dellabetta

Copy link
Copy Markdown
Collaborator

Hi @yasu-oh , so you're seeing it pretty frequently with E2B? do you have a minimal reproduction i can try on my side?

@yasu-oh

yasu-oh commented Aug 9, 2026

Copy link
Copy Markdown
Author

A further update: I can no longer reproduce the E2B failure either with my current environment:

  • compressed-tensors 0.18.1a20260806
  • llmcompressor 0.12.1.dev101+gded34c1f3

I also noticed that compressed-tensors recently disabled the Triton _quantize backend in vllm-project/compressed-tensors#817. Since the llm-compressor iMatrix grid-search code itself has not changed between the version where I originally observed the failure and my current environment, one possibility is that the CUDA error originated from the quantization backend and only surfaced later at the boolean-indexed assignment due to asynchronous CUDA execution.

Given that I can no longer reliably reproduce either the E2B or E4B issue, I don't want to overstate the evidence that the masked assignments themselves were the root cause.

That said, I still think the torch.where change may have some value as defensive hardening: it avoids the advanced-indexing update path while preserving the intended element-wise update semantics, and the added shape check provides a clearer failure mode if an unexpected tensor shape ever reaches the grid search.

I appreciate you taking the time to review and investigate this. If you think this defensive change is still worthwhile, I would be happy to keep the PR open; if you would prefer not to carry it without a currently reproducible failure, I completely understand and am also happy to close it.

@brian-dellabetta

Copy link
Copy Markdown
Collaborator

Hi @yasu-oh , thanks for the report. We disabled the triton kernels and changed some of the logic away from torch.where, to fix a regression we were hitting in nvfp4 regression tests. We will re-enable after the releases, can we revisit at that time? It'd be good to see an MRE so we can understand root cause before merging

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

two-reviews When a PR requires two reviews

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants