Skip to content

fix(adm): sum the scale-0 contrast-masking rows unsigned so a row past INT64_MAX scores - #2190

Merged
lusoris merged 1 commit into
masterfrom
fix/adm-cm-row-total-unsigned
Oct 5, 2026
Merged

lusoris merged 1 commit into
masterfrom
fix/adm-cm-row-total-unsigned

Conversation

@lusoris

@lusoris lusoris commented Oct 5, 2026

Copy link
Copy Markdown
Contributor

Summary

Two integer-ADM defects found by the RC3 integer-overflow audit of every accumulator.

1. A scale-0 contrast-masking row passes INT64_MAX and the frame fails, on every backend. The masking reduction adds, per row, the cubes ((x^2 + 2^28) >> 29) * x >> shift_cub of the CSF-weighted coefficients above their threshold. Those terms are non-negative, and the CPU (adm_cm_accum_px(), adm_cm_fold()), the AVX2 / AVX-512 rows and the CUDA, HIP and SYCL twins summed them in int64_t. Compared with itself, a picture with full-range reference detail has a zero threshold everywhere. An exact search over the column sign patterns of one band row (scratch cm_dp.py, then reproduced) puts a 31-32 px wide row at 1.044 INT64_MAX and a 63-64 px row at 1.021 at 16 bits (1.018 at 10, 1.009 at 8). The sum wraps (signed overflow), the numerator is NaN, and the frame fails with invalid ADM reduction. With a horizontal or vertical CSF weight above 38,400 (an option value) 16K overflows too. Upstream Netflix/vmaf has the same int64 row.

Fix: scale 0 sums its rows and its frame unsigned.

  • adm_cm_round_row_total_s0() and adm_cm_fold_s0(); AdmCmRowFn, adm_cm_row(), adm_cm_rows(), adm_cm_result() and the x86 rows (hsum_epu64) are uint64_t.
  • CUDA uses warp_reduce_u64() and uint64_cu rows; HIP uses an unsigned shared tree.
  • SYCL uses unsigned partials and an unsigned fold for every term (all of its terms are non-negative).
  • Metal's rows were ulong already; its host now sums uint64_t.
  • The GPU hosts read the scale-0 slots as uint64_t.

Scales 1-3 keep their signed sums: they stay 2.8x below INT64_MAX at every weight, and CUDA's i4 rounding term is negative (ADR-0155). Below 2^63 every value has the bits the signed form gave. The unsigned row holds every row at the default weights (0.52 of 2^64 at worst) and up to a horizontal or vertical weight of about 45,200. Weights between that and the ADR-1472 limit of 46,603 still take a 31-32 px row past 2^64. That case is left open as T-ADM-CM-SCALE0-ROW-UINT64-WEIGHT-BUDGET-2026-10-05, because it needs a decision: a row term in the ADR-1472 budget, or a 128-bit row on every backend.

2. The CUDA and HIP scale 1-3 decouple narrowed the gain product before bounding it. int32_t rst = (int32_t)(...) * adm_enhn_gain_limit; then min(rst, t). The CPU's adm_decouple_band_s123() bounds the double product by t and narrows the bounded value. |o| reaches 1.45e9 at scale 1, so with the default limit of 100 the product leaves int32, which makes the conversion undefined. The devices' saturating conversion happened to return t, so no score showed it. The twins now form (double)rst_q * gain, bound it with fmin / fmax and narrow once. SYCL and Metal already form the product in 64-bit integers (adm_gain_limit_product(), ADR-1413).

Evidence

On ryzen-4090-arc (correctness only, no timing claim):

Check Master 3dc36fa76 This branch
test_integer_adm_cm_row_unsigned: 64x64 picture of core/test/adm_cm_row_overflow_frame.h, 8, 10 and 16 bit, host AVX-512 dispatch and cpumask scalar frame fails (NaN numerator); a clang integer-sanitizer build reports the signed overflow at integer_adm_kernels.h:1017 and x86/adm_avx512.c:100 scores, scale0 1.0000247732 at 16 bits, same bits on both paths
test_gpu_adm_tiny_frames, new test_gpu_adm_row_past_int64_max_parity (8 and 16 bit) fails passes on CUDA (RTX 4090), HIP (gfx1036) and SYCL (Arc A380, bit for bit)
test_gpu_adm_gain_product_contract.py reports both twins passes
test_{cuda,hip,sycl}_adm_parity{,_large}, test_{cuda,hip,sycl}_adm_tiny_frames (fractional gain limits included), test_hip_adm_exact, test_{cuda,hip,sycl}_exact_twins, test_{cuda,hip}_adm_{small_border,wide_rounding,dwt2_rows} - pass
test_integer_adm_simd, _simd_noise, _tiny_frames, _cm_budget, _cm_threshold, _aim_unclipped, test_adm_cm_row_rounding, test_feature_isa_invariance - pass
test_sycl_kernel_scratch - pass (no scratch)
make test-netflix-golden - 280 passed, 3 skipped

The contract tests that pin the fold (test_adm_cm_row_rounding_contract.py, test_{cuda,hip}_adm_exact_contract.py, test_metal_integer_adm_exact_contract.py) now pin the unsigned scale-0 fold. A new planted case puts back the signed scale-0 fold and checks that the contract reports it.

tidy (container, ADR-1471): cpu integer_adm.c, x86/adm_avx2.c, x86/adm_avx512.c, test_integer_adm_cm_row_unsigned.c, test_integer_adm_simd.c 0 findings. cuda adm_cm.cu (5), adm_csf.cu (11) and cuda_helper.cuh (1) are at their baselines, with no new finding; adm_decouple_inline.cuh measures 0 against a baseline of 20. hip adm_cm.hip and integer_adm_hip.c 0, adm_csf.hip (4), adm_cm_accumulator.h (2) and adm_angle_flag.h (25) at their baselines. sycl integer_adm_sycl.cpp 0. The older CUDA/HIP debt in these files belongs to open #2109 / #2082 / #2087.

integer_adm_metal_host.c and the Metal kernel are not built on this host; the macOS tester bundle measures them.

Checklist

  • Conventional Commits.
  • docs/state.md: T-ADM-CM-SCALE0-ROW-INT64-OVERFLOW-2026-10-05 and T-GPU-ADM-S123-GAIN-PRODUCT-NARROWING-2026-10-05 opened and closed; T-ADM-CM-SCALE0-ROW-UINT64-WEIGHT-BUDGET-2026-10-05 opened.
  • No Netflix golden assertion modified; the golden gate passes.
  • Scores that did not overflow are unchanged (values below 2^63 keep their bits).

Deep-dive deliverables (ADR-0108)

  • Research digest — no digest needed: bug fix, the derivation is in the state row and in docs/development/accumulator-bounds.md of the audit PR.
  • Decision matrix — no alternatives: only-one-way fix at the default weights (the terms are non-negative and below 2^64 there; an unsigned sum keeps every existing value). The non-default weight range is left open for a decision.
  • AGENTS.md invariant note — core/src/feature/AGENTS.d/adm-rounding.md, core/src/feature/{cuda,hip}/AGENTS.d/adm.md.
  • Reproducer / smoke-test command — below.
  • CHANGELOG fragment — changelog.d/fixed/integer-adm-scale0-row-past-int64-max.md.
  • Rebase note — docs/rebase-notes.md and docs/development/rebase-sensitive-invariants.md (upstream sums int64).

Reproducer

meson test -C build test_integer_adm_cm_row_unsigned test_adm_cm_row_rounding_contract test_gpu_adm_gain_product_contract

…t INT64_MAX scores (#2190)

* fix(adm): sum the scale-0 contrast-masking rows unsigned so a row past INT64_MAX scores

The integer ADM masking reduction adds, per row, the non-negative cubes of
the CSF-weighted coefficients above their threshold, and the CPU, its
AVX2 / AVX-512 rows and the CUDA, HIP and SYCL twins summed a scale-0 row
in int64. Compared with itself, a 31-32 or 63-64 pixel wide picture with
full-range reference detail (core/test/adm_cm_row_overflow_frame.h) takes
one row to 1.044 / 1.021 INT64_MAX at the default weights: the sum
wrapped, the numerator was NaN and the frame failed with "invalid ADM
reduction". A horizontal or vertical CSF weight above 38,400 does the
same at 16K. Scale 0 now sums its rows and its frame unsigned
(adm_cm_round_row_total_s0(), adm_cm_fold_s0(), uint64_t rows in every
backend; the GPU hosts read the scale-0 slots as uint64_t), which holds
every row at the default weights (0.52 of 2^64 at worst) and up to an h/v
weight of about 45,200. Below 2^63 the values keep their bits. Scales 1-3
keep their signed sums. The weights between 45,200 and the ADR-1472 limit
of 46,603 are left open as T-ADM-CM-SCALE0-ROW-UINT64-WEIGHT-BUDGET.

The CUDA and HIP scale 1-3 decouple also narrowed the gain product
(|o| up to 1.45e9 times the limit of 100) to int32 before bounding it by
t, an undefined conversion the devices' saturation hid; they now bound
the double product with fmin / fmax and narrow once, as the CPU does.

test_integer_adm_cm_row_unsigned (8, 10, 16 bit; SIMD == scalar) and the
new row case of test_gpu_adm_tiny_frames fail on master and pass on the
CPU, CUDA, HIP and SYCL; the fold and gain contracts pin both forms; the
Netflix golden gate passes.

Closes T-ADM-CM-SCALE0-ROW-INT64-OVERFLOW-2026-10-05.
Closes T-GPU-ADM-S123-GAIN-PRODUCT-NARROWING-2026-10-05.
@lusoris
lusoris force-pushed the fix/adm-cm-row-total-unsigned branch from 9b9c01b to cf6881c Compare October 5, 2026 21:42
@lusoris
lusoris merged commit cf6881c into master Oct 5, 2026
5 of 62 checks passed
@lusoris
lusoris deleted the fix/adm-cm-row-total-unsigned branch October 5, 2026 21:42
@github-actions github-actions Bot added the type:bug Something isn't working label Oct 5, 2026
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.

2 participants