Skip to content

fix(cuda): reproduce the CPU's running float sum in psnr_hvs_cuda - #1666

Merged
lusoris merged 2 commits into
masterfrom
fix/psnr-hvs-twins-cpu-float-sum
Oct 1, 2026
Merged

lusoris merged 2 commits into
masterfrom
fix/psnr-hvs-twins-cpu-float-sum

Conversation

@lusoris

@lusoris lusoris commented Oct 1, 2026 •

Copy link
Copy Markdown
Contributor

Summary

psnr_hvs_cuda now returns the CPU extractor's scores bit for bit. On master it is up to 1.66e-2 dB from --backend cpu at 3840x2160, beyond the ADR-1361 tolerance (3.34e-3 dB), although it is the more accurate of the two.

The cause is the reference: calc_psnrhvs() adds every masked coefficient error of a plane into one running float (10.8 million terms for 3840x2160 luma), so its value depends on the order of the additions, and the twin summed per block. Per maintainer decision (2026-10-01, ADR-1397, amending ADR-1361) the twins copy the CPU's accumulation instead of widening the tolerance; a double accumulator on the CPU was ruled out because it moves a Netflix golden value past places=4. The kernel now stores the 64 terms of every block in the CPU's arithmetic and the host adds them in the CPU's order. Closes T-PSNR-HVS-CPU-FLOAT-SUM-4K-2026-09-30.

This costs throughput (numbers below), which the decision accepts: correctness first, tuning afterwards.

Type

  • feat — new feature
  • fix — bug fix
  • perf — performance improvement
  • refactor — no behavior change
  • docs — documentation only
  • test — test-only
  • build / ci — tooling / infra
  • port — cherry-pick from upstream Netflix/vmaf
  • sycl / cuda / simd — backend-specific

Checklist

  • Commits follow Conventional Commits (the commit-msg hook enforces this).
  • make format && make lint is green locally. The pre-commit hook set passed on the whole diff; the touched C files (integer_psnr_hvs_cuda.c, psnr_hvs_score.c, test_cuda_psnr_hvs_parity.c, test_psnr_hvs_score.c) measure 0 clang-tidy findings on the CUDA lane, which is their baseline, so no tidy baseline changes.
  • Unit tests pass: python3 scripts/ci/run_meson_test.py -- -C build --suite=fast on a CPU + CUDA build (RTX 4090), including test_cuda_psnr_hvs_parity, test_cuda_psnr_hvs_parity_large, test_psnr_hvs_score and test_psnr_hvs_twin_exact_sum_contract.
  • If I touched any SIMD/GPU code path, I ran /cross-backend-diff and the worst ULP is ≤ 2. The worst difference is 0: every output of every frame is bit-identical to the CPU (table below).
  • If I touched a feature extractor with SIMD/GPU twins, I either updated every twin or listed the gap under "Known follow-ups" below.
  • If I added a new .c / .cpp / .cu / .h / .hpp, it has the appropriate license header (see CONTRIBUTING.md).
  • If this is a breaking change, the commit message uses ! or BREAKING CHANGE: and the migration path is documented below.
  • If this PR adds an ADR, the ADR row lives in docs/adr/_index_fragments/<NNNN-slug>.md and the slug is appended to docs/adr/_index_fragments/_order.txt — do not edit docs/adr/README.md directly (regenerated by scripts/docs/concat-adr-index.sh; see ADR-0221).

Bug-status hygiene (ADR-0165)

  • docs/state.md updated in this PR: T-PSNR-HVS-CPU-FLOAT-SUM-4K-2026-09-30 moved to Recently closed; T-CUDA-PSNR-HVS-EXACT-SUM-THROUGHPUT-2026-10-01, T-HIP-PSNR-HVS-EXACT-SUM-2026-10-01 and T-SYCL-PSNR-HVS-EXACT-SUM-2026-10-01 opened (RC3).

Netflix golden-data gate (ADR-0024)

  • I did not modify any assertAlmostEqual(...) score in the Netflix golden Python tests.
  • If I believe a golden value must change, I have explained why below AND pinged @lusoris for a CODEOWNERS exception.

No CPU extractor code changes. --backend cpu output of this branch equals master's on all 210 frames of the fixtures below.

Cross-backend numerical results

RTX 4090 (driver 615.71.09, CUDA 13.4), --precision max, psnr_hvs on --backend cpu against psnr_hvs_cuda. Largest absolute difference in dB over the frames; "before" is master c7f28317f, "after" this branch (re-measured on head f2b637ed4, based on master 5593a70ae).

fixture (frames)                         before: psnr_hvs_y   before: psnr_hvs   after (all four outputs)
Netflix 576x324 8-bit (48)               8.37e-5              8.03e-5            0, bit-identical
Netflix 576x324 10-bit (3)               4.90e-5              4.76e-5            0, bit-identical
Netflix 576x324 12-bit (3)               3.48e-5              3.33e-5            0, bit-identical
Netflix 576x324 4:2:2 10-bit (48)        6.69e-5              6.51e-5            0, bit-identical
checkerboard 1 px 1920x1080 8-bit (3)    1.71e-3              1.71e-3            0, bit-identical
checkerboard 10 px 1920x1080 8-bit (3)   7.41e-4              7.41e-4            0, bit-identical
checkerboard 1 px 1920x1080 10-bit (3)   2.95e-4              2.95e-4            0, bit-identical
checkerboard 10 px 1920x1080 10-bit (3)  4.50e-4              4.50e-4            0, bit-identical
BBB 1920x1080 8-bit (24)                 1.38e-3              1.31e-3            0, bit-identical
BBB 1920x1080 10-bit (24)                1.73e-3              1.45e-3            0, bit-identical
BBB 3840x2160 8-bit (24)                 1.10e-2              1.05e-2            0, bit-identical
BBB 3840x2160 10-bit (24)                1.66e-2              1.45e-2            0, bit-identical

The 10-bit 1920x1080 and 3840x2160 fixtures are ffmpeg conversions of the 8-bit ones (the BBB 3840x2160 distorted side with noise=alls=6:allf=t); BBB 1920x1080 is a bicubic downscale. The parity gate on all 200 frames of BBB 3840x2160 reports a maximum difference of 0 with tolerance 0.

Each kernel change is needed. With the CPU-order sum in place, restoring one of the three remaining differences breaks identity on a few frames per fixture: nvcc contraction (up to 8.5e-7 dB), a float square root in the masking threshold (5.0e-7), a float masking table (7.4e-7). Details in Research-1397.

Performance (if perf or feat)

Slower, as expected. ms per frame on the RTX 4090, median of three runs of (t(N) − t(2)) / (N − 2), N = 960 at 576x324 (the Netflix pair concatenated 20 times), 102 at 1920x1080 and 3840x2160; other sessions shared the CPU (load average 5 to 11), the GPU was held under the device lock.

frame size   master psnr_hvs_cuda   this branch   CPU, 16 threads
576x324      0.09                   0.29          0.14
1920x1080    0.60                   3.06          1.7 to 2.9
3840x2160    2.36                   12.16         6.5

Of the 3840x2160 figure, 6.4 ms is the host sum (16.2 million additions in one dependency chain); the rest is the readback, 64.8 MB per frame instead of 1.0 MB. 15 to 32 % of a plane's terms are nonzero, so compacting them is the first tuning step; T-CUDA-PSNR-HVS-EXACT-SUM-THROUGHPUT-2026-10-01 tracks it.

Deep-dive deliverables (ADR-0108)

  • Research digest — docs/research/1397-psnr-hvs-twins-cpu-float-sum.md.
  • Decision matrix — captured in ADR-1397 ## Alternatives considered.
  • AGENTS.md invariant note — core/src/feature/cuda/AGENTS.md (the exact-sum contract of the kernel and host), core/src/feature/AGENTS.md (psnr_hvs_score.c), scripts/ci/AGENTS.md (EXACT_TWINS), and the index entry in docs/development/rebase-sensitive-invariants.md.
  • Reproducer / smoke-test command — pasted below under "Reproducer".
  • CHANGELOG fragment — changelog.d/fixed/cuda-psnr-hvs-cpu-float-sum.md.
  • Rebase note — docs/rebase-notes.md, "ADR-1397 — psnr_hvs_cuda returns the CPU's scores bit for bit".

Reproducer

ninja -C build test/test_cuda_psnr_hvs_parity test/test_psnr_hvs_score
./build/test/test_cuda_psnr_hvs_parity && ./build/test/test_psnr_hvs_score
python3 core/test/test_psnr_hvs_twin_exact_sum_contract.py
python3 scripts/ci/cross_backend_parity_gate.py --vmaf-binary build/tools/vmaf \
    --reference python/test/resource/yuv/src01_hrc00_576x324.yuv \
    --distorted python/test/resource/yuv/src01_hrc01_576x324.yuv \
    --width 576 --height 324 --features psnr_hvs --backends cpu cuda

On master the first command fails (test_psnr_hvs_cpu_cuda_identical: 3.8e-6 dB at 256x144; the 3840x2160 cases differ by 1.63e-2 and 1.66e-2 dB) and the gate reports max_abs_diff=8.370e-05 FAIL under the new contract.

Known follow-ups

  • T-CUDA-PSNR-HVS-EXACT-SUM-THROUGHPUT-2026-10-01: the exact twin is slower than sixteen CPU threads at every size measured.
  • T-HIP-PSNR-HVS-EXACT-SUM-2026-10-01: psnr_hvs_hip is unchanged here. perf(hip): upload native samples and convert on device in psnr_hvs #1658 rewrites that twin and was open when this was written; the same change goes on top of it and is verified on the gfx1036.
  • T-SYCL-PSNR-HVS-EXACT-SUM-2026-10-01: psnr_hvs_sycl is unchanged here. perf(sycl): make psnr_hvs kernel scratch-free on Arc A380 #1657 rewrites its kernel and was open when this was written. The SYCL kernel is fp64-free (ADR-0220), so the threshold's double product and square root need fp32-pair arithmetic there. Until then its gate cells keep the ADR-1361 tolerance.
  • The Metal twin is outside the parity gate's backend list and unchanged.
  • Documentation corrected on the way, checked against the source: docs/metrics/psnr-hvs.md and docs/metrics/features.md listed enable_chroma as default false or absent, psnr_hvs as the luma score and the input as 8-bit only.

Breaking changes / migration

None in the API. psnr_hvs_cuda scores change in their last digits (by up to 8.4e-5 dB at 576x324 and 1.7e-2 dB at 3840x2160) to the values --backend cpu returns.

@lusoris
lusoris force-pushed the fix/psnr-hvs-twins-cpu-float-sum branch from 4aac3fb to f2b637e Compare October 1, 2026 07:27
@github-actions github-actions Bot added the type:bug Something isn't working label Oct 1, 2026
@lusoris
lusoris force-pushed the fix/psnr-hvs-twins-cpu-float-sum branch from f2b637e to 9c300cd Compare October 1, 2026 09:18
psnr_hvs_cuda now returns the CPU extractor's scores bit for bit. Before,
it was up to 1.7e-2 dB from --backend cpu at 3840x2160, beyond the
ADR-1361 parity tolerance (3.34e-3 dB).

calc_psnrhvs() adds every masked coefficient error of a plane into one
running float, so its result depends on the order of the additions. The
twin summed the 64 terms of a block on the device and the blocks on the
host. Per maintainer decision (ADR-1397, amending ADR-1361) the twins copy
the CPU's accumulation; a double accumulator on the CPU was ruled out
because it moves a Netflix golden value past places=4.

- The kernel stores the 64 terms of every block, computed in the CPU's
  arithmetic: masking table and threshold in double, integer coefficient
  difference, fatbin built with --fmad=false.
- psnr_hvs_score.c (new) adds a plane's terms in the CPU's order and forms
  the combined score and the dB value with the CPU's expressions. It is
  built with the strict floating-point arguments of the scalar reference.
- The parity gates compare a CPU and psnr_hvs_cuda cell with tolerance 0
  at --precision max (EXACT_TWINS). The SYCL cells keep ADR-1361.
- test_cuda_psnr_hvs_parity asserts equality on all four outputs,
  including two 3840x2160 cases that fail on master by 1.6e-2 dB.
  test_psnr_hvs_score and test_psnr_hvs_twin_exact_sum_contract.py are
  device-free.

Measured on an RTX 4090 at --precision max: every frame of the Netflix
576x324 pairs (8, 10, 12 bits, 4:2:2), the 1920x1080 checkerboard pairs
and BBB 1920x1080 / 3840x2160 (8 and 10 bits) equals the CPU. CPU scores
are unchanged.

The exact sum costs throughput: 12.2 ms per 3840x2160 frame instead of
2.4 ms (1920x1080: 3.1 instead of 0.6; 576x324: 0.29 instead of 0.09),
and 256 bytes of readback per block. Tuning is tracked as
T-CUDA-PSNR-HVS-EXACT-SUM-THROUGHPUT-2026-10-01. The HIP and SYCL twins
are unchanged until #1658 and #1657 land
(T-HIP-PSNR-HVS-EXACT-SUM-2026-10-01, T-SYCL-PSNR-HVS-EXACT-SUM-2026-10-01).
Closes T-PSNR-HVS-CPU-FLOAT-SUM-4K-2026-09-30.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

type:bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant