Repository navigation
[DSV4][ROCm] Add versioned compressor state and single cache commit (task 21) - #625
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 10 issue(s) in this PR.
📄
|
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| ) | ||
| except (ImportError, AttributeError) as exc: | ||
| raise C4Error("pinned aiter C4 ABI unavailable") from exc | ||
| provider_hash = source_hash(getattr(self.aiter, "__file__", None)) |
There was a problem hiding this comment.
The pinned hash is computed from self.aiter.__file__, i.e. the package __init__.py of aiter.ops.flydsl.kernels.fused_compress_attn, not from the module that actually defines flydsl_fused_compress_attn. If the upstream layout defines the entrypoint in a sibling submodule, the pinned SHA256 either never matches (all C4 usage fails at construction) or was computed against the wrong file, which voids the ABI-pin guarantee this check is meant to provide. Pin the hash to the defining module (e.g. sys.modules[self.aiter.flydsl_fused_compress_attn.__module__].__file__) or fail closed when the entrypoint's module differs from the imported package.
| max_position = max(group * 4 for _request, group in bound.logical_keys) | ||
| require(max_position < cos.shape[0], "RoPE table misses a global C4 position") |
There was a problem hiding this comment.
The RoPE bound is checked against group * 4, the group's first token, but the kernel reads positions group*4 .. group*4+3 for the current window. A table sized exactly to max_position + 1 (i.e. cos.shape[0] == max(group*4)+1) passes this check while the last three token positions of the last group read past the end of the table. Use max(group * 4 + 3 ...) (and account for the previous-window positions (group-1)*4 for group > 0) so the bound matches the actual in-kernel position range.
|
@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 21 ??compressor state and single cache commit
Dependency: stacked on task 19 / #618 at
8793052. Target remainsdsv4-rocm. The task-21-only commit is49dc1795f3516172dbd125e6b6dfcaa25f9d1ac9; the base diff includes the prerequisite until #618 merges. Refs #512.Reuse
save_partial_statesfor FP32[kv,score+ape]history, strictly after compression.Lifecycle
Request generations, model/position versions, explicit unique writer, owned staging leases, exactly-once commit, incomplete-group handling, immutable snapshots, release/page reuse and model replacement. Duplicate/stale/foreign transactions and capacity/ownership errors fail before launch. GPU write failures poison the arena instead of silently retrying a partial commit. Device completion precedes cursor publication, avoiding early staging or page reuse across threads/streams.
Explicit C128 profile boundary
The required arithmetic handshake admits the fixed single-wave online-FP32 C128 state profile. It does not silently substitute for task 18's eight-wave HCA profile. Mismatched recipes are rejected. Full training/rollout equivalence across those different profiles is not claimed.
Real MI300X acceptance
Documentation:
docs/models/deepseek_v4/compressor-state.md.Evidence/report:
reports/compressor-state-mi300x-20261029.md.No actual distributed CP/RCCL, multi-node, full framework registration, full-model throughput, cache-write autograd or eight-wave C128 equivalence is claimed. Commit is intentionally synchronous; overlap is separate work.