Repository navigation
[DSV4][ROCm] Reuse C4 and native quantization for separate Indexer compression (task 20) - #624
inaniloquentee wants to merge 3 commits into
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the
 |
|
🔍 OpenCodeReview found 8 issue(s) in this PR.
📄
|
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
|
|
||
| def vjp(self, bound, ape, norm, cos, sin, grad, contract): | ||
| contributions, ape_rows, norm_rows = [], [], [] | ||
| cache = torch.cat((cos, sin), dim=-1) |
There was a problem hiding this comment.
cos/sin are produced for the forward path, where the kernel consumes them via cos_cache/sin_cache. Reusing them here concatenated on the last dim and passing to the pinned functional assumes an exact layout (cat vs interleave) that is not validated anywhere. If the functional expects interleaved RoPE (or a different concat order), the VJP silently computes gradients for the wrong rotation basis while forward remains correct, producing a wrong-but-plausible backward. Please confirm the expected cache layout from the pinned compressed_compact_graph signature and assert it here.
| eps=contract.rms_eps, | ||
| ) | ||
| if group > 0: | ||
| grad_out = torch.cat((torch.zeros_like(y[:1]), grad[index : index + 1].float())) |
There was a problem hiding this comment.
The cotangent layout is built by hard-coded concatenation: for group > 0 a single zero row is prepended and one grad row is passed, while for group == 0 the prefix is context[:4] and the zero padding is 4 rows. Neither the number of rows produced by functional nor the prefix extent is checked against group/ratio/contract, so any mismatch (e.g. a different prefix width or a functional returning group rows instead of group+1) yields a shape error or, worse, a silently misaligned gradient. Add an explicit assertion on y.shape[0] (and on len(grad) == len(bound.logical_keys)) before building grad_out.
| if group > 0 and (request, group - 1) not in complete: | ||
| incomplete.append(key) | ||
| continue | ||
| if entries[3][1] != rank: | ||
| unowned.append(key) | ||
| continue | ||
| selected.append(key) |
There was a problem hiding this comment.
The predecessor-completeness check uses complete (offset completeness only), while ownership is checked separately only for the current group. With cp_size>1, a group whose predecessor is complete but owned by another rank passes this check and is selected; its previous context indices are then taken from complete[(request, group-1)][offset][0] (lines 187-191), i.e. physical rows that belong to a different rank's shard. In the distributed path this silently reads unmaterialized/garbage rows (or rows that do not exist on this rank) and produces wrong overlap state for every group that follows an unowned group. The ownership test should also be applied to the predecessor before allowing selection (e.g. require entries[3][1] == rank for (request, group-1) too, otherwise treat the group as unowned/not selectable so it is emitted later by the owning rank once its context is available).
|
@inaniloquentee The automated OCR review has completed. Please review and address all automated review bot findings, then push your updates. Maintainers will begin manual review after the bot findings have been addressed. |
Roadmap task 20 ??separate Indexer C4 compression
Dependency: stacked on task 19 / #618 at
879305251c37e1b8bb0cb44409f1762752d03561. Merge #618 first. The target remainsdsv4-rocmas requested. The task-20-only change is commite61f5eeaff5f46042b381657abe0a5dd213154c9; until #618 merges, the base diff also includes that prerequisite.Reuse instead of another compressor implementation
flydsl_fused_compress_attnand nativerotate_activation, aiter MXFP4 quantization, and vLLM power-of-two FNUZ FP8 quantization. No new numerical GPU kernel or generic-matmul Hadamard fallback.MI300X acceptance
Fresh member environment: Torch
2.11.0+gitd0c8b1f, HIP7.2.53211, Triton 3.6.0, aiter 0.1.16.post3, FlyDSL 0.2.0, vLLM 0.26.0+rocm723.Documentation:
docs/models/deepseek_v4/indexer-compressor-c4.md.Full report and synchronized benchmark:
reports/indexer-compressor-c4-mi300x-20261029.md.Explicit limits
No live cache lifecycle/commit (task 22), quantized STE/full Indexer training chain (task 26), Graph/runtime integration (task 27), actual RCCL/multi-GPU CP, multi-node or full-model acceptance is claimed. These opt-in framework adapters expose a common operator boundary, not complete model registration. No end-to-end speedup claim.