dsv4/decode: consolidate per-family start_pos test sets + restore pattern default#697
Conversation
…tern default Replace the per-file `init_default_start_pos` fixtures with three canonical per-attention-family start-position sets in decode_metadata.py: `swa_decode_start_set`, `hca_decode_start_set`, `csa_decode_start_set`. - Each set is order-preserving-deduped (S=1 collapses the -seq/-1 boundary pairs; some regimes coincide at the current constants) and front-loads the 8k long-context point so it survives batch<size truncation. - Standalone SWA/HCA/CSA attention, ratio-4/128 compressors, and indexer / indexer-compressor now flip `--start-pos` and `build_tensor_specs` defaults to None so the canonical per-batch set is used again (previously the scalar DECODE_START_POS default silently bypassed the branch-covering pattern and only exercised the uniform 8k path). Explicit `--start-pos N` still overrides. - Trim redundant same-branch points: SWA drops two in-window interior samples, HCA drops the 3rd-block-crossing duplicate; CSA unchanged. - decode_layer and decode_fwd keep the uniform 8k default (integration tests). Coverage is still capped at `batch` slots (decoupling from batch is left for a follow-up); sets are kept <= batch so nothing is silently dropped at B=8.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (8)
📝 WalkthroughWalkthroughSeven DeepSeek-V4 decode test harnesses (attention CSA/HCA/SWA, compressor ratio4/ratio128, indexer, indexer compressor) are updated to remove the ChangesCanonical Decode Start-Position Sets
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request refactors the start-position logic across various DeepSeek v4 decode modules (CSA, HCA, SWA, compressors, and indexers) by introducing canonical start-position helper functions in decode_metadata.py. This replaces duplicate hardcoded patterns and updates CLI argument defaults. Feedback on the changes suggests vectorizing the _tile_starts helper function in decode_metadata.py using PyTorch operations instead of a Python loop to improve efficiency and idiomatic code quality.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| def _tile_starts(pattern: list[int], batch: int) -> torch.Tensor: | ||
| uniq: list[int] = [] | ||
| for p in pattern: | ||
| if p not in uniq: | ||
| uniq.append(int(p)) | ||
| vals = torch.empty((batch,), dtype=torch.int32) | ||
| for b in range(batch): | ||
| vals[b] = uniq[b % len(uniq)] | ||
| return vals |
There was a problem hiding this comment.
Using a Python loop to assign individual elements of a PyTorch tensor is inefficient and unidiomatic. We can vectorize this operation by creating a tensor from the unique elements, repeating it to cover the batch size, and slicing it to the exact batch size.
def _tile_starts(pattern: list[int], batch: int) -> torch.Tensor:
uniq: list[int] = []
for p in pattern:
if p not in uniq:
uniq.append(int(p))
uniq_tensor = torch.tensor(uniq, dtype=torch.int32)
return uniq_tensor.repeat((batch + len(uniq) - 1) // len(uniq))[:batch].clone()
1 participant
Summary
decode_metadata.py—swa_decode_start_set,hca_decode_start_set,csa_decode_start_set— replacing the per-fileinit_default_start_posfixtures.-seq/-1boundary pairs; some regimes coincide at the current constants) and front-loads the 8k long-context point so it survivesbatch < set-sizetruncation.--start-posandbuild_tensor_specsdefaults toNonein the 7 standalone SWA/HCA/CSA attention, ratio-4/128 compressor, and indexer / indexer-compressor files, so the canonical per-batch set is used again. Previously the scalarDECODE_START_POSdefault silently bypassed the branch-covering pattern and only exercised the uniform 8k path. Explicit--start-pos Nstill overrides.decode_layeranddecode_fwdkeep the uniform 8k default (integration tests).Coverage is still capped at
batchslots (decoupling from batch is left for a follow-up); sets are kept<= batchso nothing is silently dropped at B=8.