Skip to content

Check IQ CUDA encoder ties per block instead of by count - #2708

Open
LinCanNerd wants to merge 1 commit into
NVIDIA:mainfrom
LinCanNerd:fix/iq-cuda-pack-tie-test
Open

LinCanNerd wants to merge 1 commit into
NVIDIA:mainfrom
LinCanNerd:fix/iq-cuda-pack-tie-test

Conversation

@LinCanNerd

@LinCanNerd LinCanNerd commented Oct 8, 2026 •

Copy link
Copy Markdown
Contributor

What does this PR do?

Type of change: Bug fix (tests)

Fixes #2705.

test_cuda_pack_reconstruction_matches_pytorch_at_scale[iq1_m] fails on Jetson Thor because 2 of 1024 blocks differ from the PyTorch encoder, and the bound is blocks // 1000 = 1. Both blocks are exact ties: the CUDA and PyTorch encodings decode to different values but reconstruct the block with bit-identical squared error. As the docstring of test_cuda_pack_matches_pytorch_encoder_and_is_decodable explains, such near-ties round differently because the two encoders fuse the same arithmetic differently. How often that happens depends on how the compiler fuses the CUDA encoder's arithmetic for the target GPU, so the count bound encodes one GPU's tie rate. The property the test is after ("may disagree on a near-tied scale, but not on quality") is not what it checks.

The test now checks that property directly:

  • Every block whose bytes differ must reconstruct with the same squared error as the reference (rtol=1e-5).
  • The total-error check stays.
  • The count bound is loosened to 1% of blocks.

The neighbouring docstring now says the tie rate depends on the GPU.

Usage

No API change.

Testing

On Jetson Thor (sm_110, JetPack 7.2.1, torch 2.13.0+cu130):

  • pytest tests/gpu/torch/quantization/test_iq_formats_cuda.py: 61 passed. Before the change, the iq1_m case failed.
  • Negative check: flipping one grid-index bit in a single packed block (at two different byte offsets, for iq1_m and iq2_xs) makes the updated test fail in all four cases. It still catches a real quality difference.
  • pre-commit run --files tests/gpu/torch/quantization/test_iq_formats_cuda.py: passed.

Before your PR is "Ready for review"

Make sure you read and follow Contributor guidelines and your commits are signed (git commit -s -S).

Make sure you read and follow the Security Best Practices (e.g. avoiding hardcoded trust_remote_code=True, torch.load(..., weights_only=False), pickle, etc.).

  • Is this change backward compatible?: ✅
  • If you copied code from any other sources or added a new PIP dependency, did you follow guidance in CONTRIBUTING.md: N/A
  • Did you write any new necessary tests?: N/A (test fix)
  • Did you update Changelog?: N/A (tests only)
  • Did you get Claude approval on this PR?: N/A

Additional Information

Found while running tests/gpu/torch/quantization and tests/gpu/torch/kernels on Jetson Thor. The only other Thor-specific failure is #2706, which has its own PR. #2686 also edits this test file, but only appends tests after line 226, so the two changes don't overlap.

Summary by CodeRabbit

  • Tests
    • Updated quantization parity checks to account for small scale-rounding differences across blocks while comparing CUDA and PyTorch reconstruction errors at both block and overall levels. This makes the test criteria better reflect observed numerical variation.

test_cuda_pack_reconstruction_matches_pytorch_at_scale[iq1_m] fails on
Jetson Thor: 2 of 1024 blocks differ from the PyTorch encoder, over the
blocks // 1000 = 1 bound. Both are exact ties: the differing encodings
reconstruct their block with bit-identical squared error. How often such
near-ties round differently depends on how the compiler fuses the CUDA
encoder's arithmetic for the target GPU, so the count bound encodes one
GPU's rate rather than the property the test is after.

The test now requires every differing block to reconstruct as well as
the reference, keeps the total-error check, and loosens the count bound
to 1%. Flipping a grid-index bit in one block still fails it.

Signed-off-by: LinCanNerd <lincanecdl@gmail.com>
@LinCanNerd
LinCanNerd requested a review from a team as a code owner October 8, 2026 10:14
@copy-pr-bot

copy-pr-bot Bot commented Oct 8, 2026

Copy link
Copy Markdown

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

@coderabbitai

coderabbitai Bot commented Oct 8, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Repository: NVIDIA/Model-Optimizer/.coderabbit.yaml
  • Review profile: CHILL
  • Plan: Enterprise
  • Run ID: 841d59c0-1c51-42c9-b41f-dc02379eae07
📥 Commits

Reviewing files that changed from the base of the PR and between 90ba9fb and 731a21c.

📒 Files selected for processing (1)
  • tests/gpu/torch/quantization/test_iq_formats_cuda.py

Included review availability: This review used your included allowance. Your plan provides up to 12 included reviews per hour; 10 remain after this review.


📝 Walkthrough

Walkthrough

The CUDA IQ-format parity test now compares reconstruction squared error per 256-value block, allows more differing packed blocks, and checks per-block and total errors against PyTorch.

Changes

CUDA IQ reconstruction parity

Layer / File(s) Summary
Per-block reconstruction parity
tests/gpu/torch/quantization/test_iq_formats_cuda.py
The test allows differing packed blocks up to blocks // 100. For differing blocks, it compares CUDA and PyTorch squared errors at rtol=1e-5, and applies the same tolerance to summed errors. The comment updates the stated frequency of scale-rounding differences.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix · Severity of issue fixed: Low

Suggested reviewers: cjluo-nv

Merge Risk: ⚪ Minimal · up to 731a2

This test-only change checks the intended reconstruction-error parity, with no identified merge-blocking risk.

🚥 Pre-merge checks | ✅ 6
✅ Passed checks (6 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly describes the main change: the IQ CUDA test now checks per-block reconstruction-error ties instead of relying only on a differing-block count.
Linked Issues check ✅ Passed The PR meets the coding requirements in #2705. In tests/gpu/torch/quantization/test_iq_formats_cuda.py, it computes squared error for each 256-value block, compares every differing block with `torch…
Out of Scope Changes check ✅ Passed The diff is limited to the CUDA IQ format test file. The changes update the test explanation and its reconstruction assertions. These changes directly support the test-only objective in #2705. No unre…
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 2 functions across 1 files.
Security Anti-Patterns ✅ Passed PASS. The PR changes only tests/gpu/torch/quantization/test_iq_formats_cuda.py. The added lines contain no torch.load(..., weights_only=False), numpy.load(..., allow_pickle=True), hardcoded `tru…
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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

This branch has not been deployed

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

test_cuda_pack_reconstruction_matches_pytorch_at_scale[iq1_m] fails on Jetson Thor: exact ties exceed the blocks // 1000 bound

1 participant