Conversation
|
Could you provide more details about the current test results? |
|
@claude review this PR |
|
Claude finished @nullnonenilNULL's task in 3m 14s —— View job PR Review: [Task] INT4 QAT SupportTasks
Verdict: 🟠 Request ChangesSummary: The INT4 QAT implementation is well-structured with solid CUDA kernels, a fused fast path, and a clean STE-based weight-transform approach. The main concerns are: a correctness bug in the asymmetric STE path (gradient is returned for the wrong dtype), a missing Tests: Unit tests are thorough and cover symmetric, asymmetric, bfloat16, fused, and STE paths. The Checklist
Findings🔴 Critical —
|
…refresh docs(readme): add architecture diagram and Architecture section
Add INT4 Quantization-Aware Training (QAT) support for MoE expert linear layers. - Add int4_qat CUDA kernels (fake quantization/dequantization) - Add CLI arguments: --enable-int4-qat, --int4-qat-group-size, --int4-qat-filter-regex - Add QAT setup in training_utils.py pretrain loop - Update Dockerfile to install int4_qat package - Update .gitignore for int4_qat build artifacts
57181cb to
be2ddcc
Compare
- Add SPDX license headers to int4_qat sources; use // comment style for .cu files (# is an invalid C preprocessor directive) - Split spdx-check pre-commit hook: keep # style for py/sh, add a // style entry for cu/cpp/h under the same alias - Fix unused imports in int4_qat package, setup.py and tests - Clean up pre-existing ruff violations in training_utils.py so the lint gate (which checks whole changed files) passes Change-Id: I2e4bd089c24dcb785355fda81f7d96d5ccc425b7
Apache-2.0 requires retaining copyright attribution in derivative works. Add the standard 'Modified from' block used elsewhere in the repo, citing slime (THUDM/slime, Copyright 2025 Zhipu AI). Change-Id: If1cf41b8274d96498952d200529a8d37a6272745
Partial blocks are zero-padded to the full block size in the CUDA kernels (inherited from slime) and the pure-PyTorch fallback, with the padding participating in min/max range estimation. The loop-based test reference reduced over true blocks only, so test_quant_asymmetric failed on partial-tile shapes ((7,65), (65,7), (9,257)). - Pad partial blocks in _reference_fake_int4_quant to match kernel behavior (verified exact match against lane-level kernel emulation for all test shapes, sym and asym) - Document the semantics in fake_int4_quant docstring, including the TP-sliced weight case; symmetric mode is unaffected (|0| never changes a block's abs-max) Change-Id: I7055abeeb3abdc5d356c0302b1cab83d832816b0
Zero-padding is only legitimate at the true tail of a tensor. When a weight is TP-sharded along in_features and the local shard size is not a multiple of group_size, a quantization group would straddle the TP boundary and each rank would pad with zeros that are real values on another rank, producing wrong scales in both sym and asym modes. Handling that correctly requires a cross-rank amax reduction (like Megatron's master-weight quantization across DP ranks), which is not implemented. - Validate group alignment at patch time via Megatron's tensor_model_parallel/partition_dim parameter attributes; raise with a remedy message on straddle (fail fast at setup, not mid-step) - Allow and log genuine tail padding on unsharded tensors - Add TestGroupAlignment tests (CPU-only); document the requirement in the README and the fake_int4_quant docstring Change-Id: If8d17ddddcb95816d38d2926acce1727563b402f
Add INT4 Quantization-Aware Training (QAT) support for MoE expert linear layers.
Resolves #12