Skip to content

fix(quant): calibrate int8 and AWQ low-memory runs on every sample, not one per batch - #388

Merged
yghstill merged 3 commits into
Tencent:mainfrom
MincongZhou:fix/quant-low-memory-nsamples
Oct 31, 2026
Merged

yghstill merged 3 commits into
Tencent:mainfrom
MincongZhou:fix/quant-low-memory-nsamples

Conversation

@MincongZhou

Copy link
Copy Markdown
Contributor

Problem

The calibration-setup step that sizes the captured-activation buffer exists three times in this package, and only the fp8 copy got it right:

file:line expression
correct angelslim/compressor/quant/modules/fp8/fp8.py:78 len(dataloader) * dataloader.batch_size
wrong angelslim/compressor/quant/modules/int8/int8.py:77 len(dataloader)
wrong angelslim/compressor/quant/modules/awq/awq.py:125 len(dataloader)

Catcher stores one entry per sample — its own docstring on _capture_samples says "Splits a batched input into per-sample tensors", and the loop is for i in range(inp.shape[0]): ... captured_inputs.append(sample). So len(captured_inputs) is len(dataloader) * batch_size.

int8/awq then allocate self.inps with the smaller len(dataloader) and copy with

for idx in range(min(len(captured_inputs), self.inps.shape[0])):   # int8.py:92

which silently discards every sample past the first batch, before any observer sees it. Two consequences:

  1. The activation scales for int8 and AWQ are calibrated on 1/batch_size of the requested data.
  2. print_info(f"nsamples:{nsamples}") reports the truncated number, so the run log states the wrong calibration-set size — the only signal the user gets is a number that looks intentional.

Reproduction

Executed against the real Catcher from this repo (96-sample dataset, batch_size=8, so len(dataloader)=12):

dataset: 96 samples, batch_size=8 -> len(dataloader)=12
Catcher captured (per-sample)          : 96
int8  inps rows = 12   fill loop keeps 12   -> DROPS 84 samples before any observer sees them
awq   inps rows = 12   fill loop keeps 12   -> DROPS 84 samples
fp8   inps rows = 96   fill loop keeps 96   -> correct

--- after this change ---
int8  inps rows = 96   fill loop keeps 96   -> DROPS 0 samples
awq   inps rows = 96   fill loop keeps 96   -> DROPS 0 samples
fp8   inps rows = 96   fill loop keeps 96   -> correct

The same input yields 12 calibrated samples on int8/AWQ and 96 on fp8 today.

Provenance

git log on the two files says this is a migration leftover, not an intentional difference:

So the fix is to make int8 and AWQ agree with fp8.

Scope

nsamples is used only as an allocation length and as a min() bound, so this cannot index out of range. At batch_size = 1 the two expressions are equal — and every low_memory: true config shipped under configs/ uses batch_size: 1 — so this is a no-op for the existing recipes and only changes runs that ask for batched calibration, which is exactly the case that is broken today. dataloader.batch_size is already dereferenced unconditionally by the fp8 copy.

Validation

  • Two-line change, no control flow touched.
  • python -m py_compile on both files: OK.
  • The repository's own pre-commit tools, at the pinned versions from .pre-commit-config.yaml:
    • black --line-length 99 --check → 2 files would be left unchanged
    • isort --profile=black --check-only → clean
    • flake8 --ignore=E203,E704,W503,W504 --max-line-length 99 → exit 0
  • The reproduction above, before and after, using the real Catcher class imported from this repository.

Not run: an end-to-end PTQ to compare the resulting quantized accuracy (needs model weights, a real calibration dataset and a GPU). The claim here is limited to what the code does: the buffer is sized for one batch instead of the whole capture set, and the surplus is dropped before the observer loop.

Competition

No issue or PR mentions this. gh api search/issues for int8 low_memory → 0 results; for nsamples → 2 (the fp8 fix #29 and an unrelated #318). The 4 currently open PRs (#351, #346, #319, #305) touch other areas. The one open issue in this neighbourhood (#48) is about FP8 low-memory and W4A8 and does not mention sample counting.

…ot one per batch

The calibration-setup step exists three times and only the fp8 copy is right:

  fp8/fp8.py:78   nsamples = len(dataloader) * dataloader.batch_size
  int8/int8.py:77 nsamples = len(dataloader)                  <- drops all but the last batch
  awq/awq.py:125  nsamples = len(dataloader)                  <- same

Catcher stores one entry per sample (see the docstring on _capture_samples:
"Splits a batched input into per-sample tensors"), so len(captured_inputs) is
len(dataloader) * batch_size. But int8/awq size self.inps with len(dataloader)
and copy with min(len(captured_inputs), self.inps.shape[0]), so every sample
past the first batch is discarded before an observer sees it: with 96 samples
at batch_size 8 the int8 and AWQ activation scales are calibrated on 12 of them,
while print_info("nsamples:12") reports the truncated number as if it were the
requested one. The fp8 copy has measured the full set since Tencent#29.

Sizing the buffers by the real capture count makes the three modules agree and
lets the observer see every sample. At batch_size 1 -- every low_memory config
shipped in configs/ -- the two expressions are equal, so this is a no-op for the
existing recipes and only changes runs that ask for batched calibration.
INT8.low_memory_run and AWQ.run size self.inps from nsamples and then copy the
inputs Catcher captured into it, capped by min(len(captured_inputs),
self.inps.shape[0]). Sizing the buffer from len(dataloader) - one row per batch
- therefore keeps only that many samples and silently drops the rest, so the
low-memory calibration saw a fraction of the corpus while reporting success.

Drives both real methods with a stubbed model: 3 batches of 8 means the buffer
must be 24 rows with every captured sample copied into it, not 3. Only the
modules under test are loaded, with their imports stubbed, following
tests/test_dataloader.py.
yghstill
yghstill previously approved these changes Oct 14, 2026
@yghstill

yghstill commented Oct 15, 2026 •

Copy link
Copy Markdown
Collaborator

@MincongZhou Please refacing code format:

pre-commit install
pre-commit run --all-files

@MincongZhou

Copy link
Copy Markdown
Contributor Author

Applied the repo's own pre-commit config: black 24.4.0 --line-length=99, isort 5.13.2 --profile=black, flake8 7.0.0.

The diff is whitespace-only in tests/test_quant_low_memory_nsamples.py (12+/6-): three over-long lines rewrapped, no semantic change. Verified the whole tree locally — 485 files pass black / isort / flake8.

Heads-up: the two workflows on the new head (fd50303) are sitting in action_required, so they need approval from a maintainer before they start.

@yghstill
yghstill merged commit 44a0664 into Tencent:main Oct 31, 2026
5 checks passed
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.

2 participants