Repository navigation
test(accel): add KIVI reference implementation and CPU boundary regressions - #658
DoubleCore wants to merge 2 commits into
Conversation
…ssions Refs inclusionAI#590. Incremental verification artifact: CPU-only reference implementation plus regression tests. Does not modify the mainline implementation.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b1a21d88b7
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
|
||
| .. code-block:: console | ||
|
|
||
| C:/Users/AORUS/anaconda3/python -m pytest tests/test_kivi_reference_cpu.py -q |
There was a problem hiding this comment.
On Linux or any workstation without this particular Windows Anaconda installation, the documented reproduction command cannot launch the tests. Replace the absolute interpreter path with an environment-selected interpreter, such as python, so contributors can reproduce the verification. The root AGENTS.md explicitly prohibits hardcoded absolute paths.
AGENTS.md reference: AGENTS.md:L35-L35
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 83df034: the reproduction command now uses python. CPU-only verification: python -m pytest tests/test_kivi_reference_cpu.py -q passed all 26 tests on torch 2.7.1+cpu with CUDA unavailable.
| self._value_blocks.append( | ||
| quantize_value_per_token( | ||
| values[:, :, start:stop], bits=self.bits, group_size=self.group_size | ||
| ) |
There was a problem hiding this comment.
With group_size=4, residual_length=2, and head_dim=3, appending two tokens succeeds, but appending four more adds a key block before this value quantizer raises because the head dimension is not divisible by the group size. The failed update leaves num_tokens == 2, quantized_tokens == 4, and no corresponding value block, so materialize() subsequently returns different key/value lengths. Validate head-dimension divisibility in _validate_update before changing metadata or block lists.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 83df034. _validate_update now rejects a head_dim that is not divisible by group_size before metadata or block lists are mutated. The new regression test verifies that rejection leaves token counts, block lists, residual state, and empty-cache materialization behavior unchanged. CPU-only suite: 26 passed; no GPU validation was performed.
|
Implemented both review fixes in
CPU-only verification with torch 2.7.1+cpu (CUDA unavailable):
No GPU/CUDA validation was performed. |
Adds an independent CPU reference implementation for KIVI-style 2-bit KV cache quantization and boundary regression tests.
This is an incremental verification artifact and does not modify the mainline implementation (PR #641).
What is covered
Verification
C:/Users/AORUS/anaconda3/python -m pytest tests/test_kivi_reference_cpu.py -q→ 25 passedNot covered
CUDA memory savings and real-hardware throughput (CPU-only environment). Flagged explicitly in the report at
docs/dev/kivi-reference-verification.rst.Refs #590