feat(codec): PR-X12 v1 — λ-RDO mode selection + rANS entropy coder#211
Conversation
Completes the x265-shaped cognitive codec to a usable v1 by adding the two stages the board tracked as missing (RDO + ANS): - `codec::rdo` (A6) — λ-rate-distortion mode selection: scores all four modes by `(rate << 8) + λ_q8·distortion` and picks the minimum. The soft, cost-weighted generalization of `predict_intra`'s hard tree. Uses an integer fixed-point λ (λ × 256) rather than the design's f32, for deterministic, cross-platform-bit-exact decisions consistent with the substrate's no-float discipline. - `codec::ans` (A7) — static-table rANS over the 4-symbol mode alphabet (Skip/Merge/Delta/Escape). Self-contained stream (count + normalized freq table + payload); bit-exact round-trip. Chosen over CABAC per the design's open-Q1 ruling. Grounded against the existing integer-only LeafCu/CellMode/MergeDir and the 2/3/3/6-byte wire format; transform (A4) stays deferred to v2 per design Q2, stream (A8) is the remaining follow-on. 81 lib tests + 20 doctests green; clippy -D warnings clean. https://claude.ai/code/session_01HbqooFZHAjaUtFEzhA1R2u
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds a fixed-point RDO selector for per-cell mode choices and a static-table rANS encoder/decoder for the 4-symbol mode stream, plus module wiring and a blackboard status update. ChangesRDO and rANS Codec Implementation
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 52e44f5af7
ℹ️ 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".
| let hi = stream[5 + 2 * i]; | ||
| *slot = u16::from_le_bytes([lo, hi]); | ||
| } | ||
| let table = RansFreqTable::from_freqs(freq)?; |
There was a problem hiding this comment.
Reject empty frequency tables for nonempty streams
When the header says n > 0 but all four stored frequencies are zero, from_freqs still succeeds here and rans_decode then fabricates Escape symbols from a zero-frequency table instead of treating the stream as malformed. This lets a corrupt/non-encode_modes stream with a nonempty count and an empty model decode to bogus mode tags; add a n == 0/sum consistency check before decoding.
Useful? React with 👍 / 👎.
Codex P2: decode_modes accepted a stream whose header claimed n > 0 while all four stored frequencies were zero (an "empty model"). from_freqs permits an all-zero table for the n == 0 empty-stream case, so a corrupt or non-encode_modes stream slipped through and rans_decode fabricated Escape tags from the freq-0 table instead of reporting malformed input. Add an explicit n > 0 / all-zero-table guard + regression test. https://claude.ai/code/session_01HbqooFZHAjaUtFEzhA1R2u
Summary
Completes the x265-shaped cognitive codec (
ndarray::hpc::codec) to a usable v1 by adding the two stages the board tracked as the outstanding PR-X12 debt — RDO and ANS:codec::rdo(A6) — λ-rate-distortion mode selection. Scores all four modes bycost = (rate_bytes << 8) + λ_q8 · distortionand picks the minimum. It is the soft, cost-weighted generalization ofpredict_intra's hard decision tree. Uses an integer fixed-point λ (lambda_q8= λ × 256) instead of the design doc'sf32, giving deterministic, cross-platform-bit-exact mode decisions consistent with the substrate's no-float discipline.codec::ans(A7) — static per-block rANS entropy coder over the 4-symbol mode alphabet (Skip/Merge/Delta/Escape). Self-contained stream layout (symbol count + normalized 4×u16 freq table + rANS payload); bit-exact round-trip. Chosen over CABAC per the design's open-Q1 ruling (cache-friendlier, no per-bit context branches).Both stages are grounded against the existing integer-only
LeafCu/CellMode/MergeDirand the 2/3/3/6-byte wire format (mode.rs). Nounsafe, no float in either module.Scope boundary (per the design doc):
transform(A4) stays deferred to v2 — open-Q2 ruled a 1-D DCT doesn't help an 8-bit scalar residual.stream(A8, byte-stream framing overans) is the remaining follow-on.Test plan
cargo test -p ndarray --features std,codec --lib hpc::codec— 81 lib tests pass (incl. rANS round-trip on empty/single/all-same/mixed/large-random/skewed streams; RDO λ-sweep, merge-closest, escape-cursor, cost-formula).cargo test -p ndarray --features std,codec --doc hpc::codec— 20 doctests pass.cargo clippy -p ndarray --features std,codec --lib -- -D warnings— clean.cargo fmt— clean.https://claude.ai/code/session_01HbqooFZHAjaUtFEzhA1R2u
Generated by Claude Code
Summary by CodeRabbit
New Features
Documentation
Tests