Repository navigation
fix(adm): keep the scale-0 masking centre tap in int32 (ADR-1402) - #1700
Merged
Merged
Conversation
lusoris
force-pushed
the
fix/adm-cm-centre-tap-wrap
branch
from
October 1, 2026 13:16
f0b4190 to
0fda806
Compare
No score changes. The scalar integer ADM kernels move from integer_adm.c into integer_adm_kernels.h, and adm_avx2.c / adm_avx512.c call them for edge columns, vector tails and the per-row fold instead of carrying their own macro copies. Every function in the two x86 files and in the SYCL DWT launches now fits the 60-line limit, so the files can be edited under the touched-file rule (ADR-1298) without a waiver. The x86 contrast-masking tails previously expanded the upstream ADM_CM_ACCUM_ROUND macro, which left-shifts a negative threshold. They now run adm_cm_accum_px(), which already uses adm_cm_excess_s0(), so the undefined shift is gone on every dispatch level. Checked bit for bit against the previous objects: scalar, AVX2 and AVX-512 stage outputs on 14976 generated cases (plus filled buffers, non-default viewing distance, gain limits and p-norm), and CLI JSON at --precision max on 15 picture pairs for cpumask 0, 48 and the default. The SYCL kernels match the scalar path on the OpenCL CPU device and the refactored DWT kernels use no private or spill memory on the Arc A380.
Integer ADM no longer scores isolated impairments above 1, and no Netflix golden assertion moves. The scale-0 masking threshold narrowed its centre tap to int16, as upstream master does; a coefficient of 15360 or more wrapped it negative, and a negative threshold adds contrast instead of masking it. The tap is now int32 and the excess over the threshold is clamp(|x| - thr * 2^shift, 0, INT32_MAX), formed in int64, in the scalar, AVX2, AVX-512, CUDA, HIP, SYCL and Metal code. This is the second revision of Netflix/vmaf PR #1602, which upstream has not merged, taken by maintainer decision on the condition that the goldens hold (ADR-1402). Golden gate (gcc golden profile): 271 passed, 12 skipped before and after. The 13 fixture pairs give identical JSON at --precision max on the CPU and on the CUDA (RTX 4090), HIP (gfx1036) and SYCL (Arc A380) twins. What moves, on every dispatch level and twin alike: a flat 64x64 reference against isolated patches, integer_adm_scale0 1.0829225419556654 to 1 and integer_adm2 1.035481944668303 to 1; independent full-range noise at 576x324, integer_adm2 0.38954843646923215 to 0.38950305912838107. The fork differs from upstream master on such content until #1602 merges. The vector kernels equal the scalar on every operand: a row is summed with the 32-bit form, which is exact while every threshold lies in [0, 2^19), and summed again with an exact form otherwise. Upstream's own vector form differs from its scalar for a negative threshold and is not used. The leftover columns of a row are the top lanes of one more vector block, so the x86 kernels have no scalar tail left to shift a negative threshold in. Stage time of the scale-0 contrast masking against the unsplit files: scalar +7%, AVX2 -5%, AVX-512 +5%; the AVX-512 pipeline as a whole is about 3% slower, which is tracked as T-ADM-AVX512-SPLIT-STAGE-TIME-2026-10-01. No snapshot under testdata/ changes: they are computed from the Netflix pairs. The Metal twin is changed in source only; no Apple device was available. Opened: T-ADM-DECOUPLE-X86-FRACTIONAL-GAIN-ROUNDING-2026-10-01.
lusoris
force-pushed
the
fix/adm-cm-centre-tap-wrap
branch
from
October 1, 2026 14:00
0fda806 to
0fb4b27
Compare
17 of 26 tasks
lusoris
added a commit
that referenced
this pull request
Oct 1, 2026
The x86 SIMD kernels are bit-exact twins of scalar references that live in baseline libraries, where no fused multiply-add exists. Most kernels finish the last n % lanes elements in plain C, and the two general libraries (x86_avx2, x86_avx512) were built with vmaf_fp_model_args only. Under icx that is -fp-model=precise, which implies -ffp-contract=on, so an icx build (every SYCL build is one) rounded those tails once where the reference rounds twice. In ssim_avx512.c that moved a score. Measured on an AVX-512 host (9950X3D, icx 2026.0) at --precision max: the CPU float_ms_ssim of an icx build differed from a GCC build and from its own scalar path by one fp32 unit in a per-scale mean, 7.7e-9 to 1.4e-8 in the score, on 4 of 104 frames (Netflix 576x324 pair, 1080p checkerboards, BBB 3840x2160). adm, vif and speed files were contracted too (42, 2 and 43 to 47 FMA instructions against 0, 0 and 3 with GCC), with no score difference measured. ADR-1415: - core/src/meson.build: both general libraries take vmaf_strict_fp_args, like the nine carve-out libraries. - test_strict_fp_compiler_args.py: both are strict targets. - test_ssim_x86_simd (new): AVX2 and AVX-512 precompute, variance and accumulate against transcriptions of the scalar reference, bit for bit, at element counts with and without a tail. Fails on the unfixed icx build at ssim_variance_avx512 n=1. - test_integer_adm_simd takes _simd_strict_fp_args: it compiles the scalar ADM kernels into its own translation unit and failed on icx builds since #1700 (the compiler's default fast model). GCC: the disassembly of all 28 objects of the two libraries is identical with and without the flag, so GCC builds compute what they did. icx: only explicit fmadd intrinsics and fmaf() calls remain; fast suite 207 of 207 (device suites left out), simd suite 21 of 21. GCC build against icx build, --backend cpu, 19 features on four fixtures: 15 bit-identical; psnr, psnr_hvs, ciede and speed_chroma still differ (at most 7.1e-15, 7.1e-15, 5.7e-12, 1.2e-6) because icx links Intel's math library. docs/state.md: T-ICX-SSIM-AVX512-FP-CONTRACT-2026-10-01, T-ICX-X86-SIMD-GENERAL-LIBS-CONTRACT-2026-10-01 and T-ICX-INTEGER-ADM-SIMD-TEST-FP-MODEL-2026-10-01 found and closed; T-ICX-LIBIMF-HOST-MATH-2026-10-01 opened.
lusoris
added a commit
that referenced
this pull request
Oct 1, 2026
* fix(build): build every x86 SIMD library without FP contraction The x86 SIMD kernels are bit-exact twins of scalar references that live in baseline libraries, where no fused multiply-add exists. Most kernels finish the last n % lanes elements in plain C, and the two general libraries (x86_avx2, x86_avx512) were built with vmaf_fp_model_args only. Under icx that is -fp-model=precise, which implies -ffp-contract=on, so an icx build (every SYCL build is one) rounded those tails once where the reference rounds twice. In ssim_avx512.c that moved a score. Measured on an AVX-512 host (9950X3D, icx 2026.0) at --precision max: the CPU float_ms_ssim of an icx build differed from a GCC build and from its own scalar path by one fp32 unit in a per-scale mean, 7.7e-9 to 1.4e-8 in the score, on 4 of 104 frames (Netflix 576x324 pair, 1080p checkerboards, BBB 3840x2160). adm, vif and speed files were contracted too (42, 2 and 43 to 47 FMA instructions against 0, 0 and 3 with GCC), with no score difference measured. ADR-1415: - core/src/meson.build: both general libraries take vmaf_strict_fp_args, like the nine carve-out libraries. - test_strict_fp_compiler_args.py: both are strict targets. - test_ssim_x86_simd (new): AVX2 and AVX-512 precompute, variance and accumulate against transcriptions of the scalar reference, bit for bit, at element counts with and without a tail. Fails on the unfixed icx build at ssim_variance_avx512 n=1. - test_integer_adm_simd takes _simd_strict_fp_args: it compiles the scalar ADM kernels into its own translation unit and failed on icx builds since #1700 (the compiler's default fast model). GCC: the disassembly of all 28 objects of the two libraries is identical with and without the flag, so GCC builds compute what they did. icx: only explicit fmadd intrinsics and fmaf() calls remain; fast suite 207 of 207 (device suites left out), simd suite 21 of 21. GCC build against icx build, --backend cpu, 19 features on four fixtures: 15 bit-identical; psnr, psnr_hvs, ciede and speed_chroma still differ (at most 7.1e-15, 7.1e-15, 5.7e-12, 1.2e-6) because icx links Intel's math library. docs/state.md: T-ICX-SSIM-AVX512-FP-CONTRACT-2026-10-01, T-ICX-X86-SIMD-GENERAL-LIBS-CONTRACT-2026-10-01 and T-ICX-INTEGER-ADM-SIMD-TEST-FP-MODEL-2026-10-01 found and closed; T-ICX-LIBIMF-HOST-MATH-2026-10-01 opened. * docs: regenerate the indexes and the citation map after rebasing
This was referenced Oct 1, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Integer ADM no longer scores isolated impairments above 1, and no Netflix golden assertion moves: the golden gate gives 271 passed, 12 skipped before and after.
The scale-0 masking threshold narrowed its centre tap to int16, as upstream master does. A coefficient of 15360 or more wrapped the tap negative, and a negative threshold adds contrast instead of masking it. The tap is now int32 and the excess over the threshold is
clamp(|x| - thr * 2^shift, 0, INT32_MAX), formed in int64, in the scalar, AVX2, AVX-512, CUDA, HIP, SYCL and Metal code. This is the second revision of Netflix/vmaf PR 1602, which upstream has not merged; the maintainer chose to take it now on the condition that the goldens hold (ADR-1402,docs/adr/1402-adm-cm-centre-tap-int32.md).Base:
origin/master. PR #1662 merged while this was in progress, so the branch is not stacked on it.Two commits, meant to be read separately:
refactor(adm): no score change.adm_avx2.c,adm_avx512.cand the SYCL DWT launches are split to the 60-line limit, which the commit hook requires before those files can be edited (ADR-1298). The scalar kernels move frominteger_adm.cintointeger_adm_kernels.h, and the x86 files call them instead of expanding their own copies of upstream's macros.fix(adm): the change above, its tests and its documentation.Type
fix— bug fixrefactor— no behavior change (first commit)sycl/cuda/simd— backend-specificWhat changes score, and what does not
CPU, default options, pooled mean. The values are the same at
--cpumask0, 48 and 4294967295, and the CUDA, HIP and SYCL twins move by the same amounts.integer_adm_scale0integer_adm2integer_adm_scale0integer_adm2integer_adm2integer_adm_scale0integer_aiminteger_adm3Unchanged at
--precision max: the three Netflix pairs and the other ten fixture pairs underpython/test/resource/yuv/(adm,float_adm, and the default model where the frame is large enough for it), one-pixel stripes, impulses on a gradient and blurred blocks. The default model's own ADM features and its VMAF score did not move on the two noise pairs either.float_admgives exactly 1 on both patch pictures.Until upstream merges PR 1602, the fork's integer ADM differs from upstream master on content that reaches a centre coefficient of 15360.
Cross-backend numerical results
20 picture pairs (13 fixture pairs, 7 synthetic),
--precision max. "Before" is a build of master2c3acf1c9.--cpumask 48)adm_cuda, RTX 4090adm_hip, gfx1036adm_sycl, Arc A380 (xe)integer_adm_metalThe SYCL ADM kernels still use no scratch memory on the A380 (
test_sycl_kernel_scratch: 108 kernels audited, the 2 that use scratch are in the ratchet, neither is integer ADM), so the A380 results count.Kernel level, old objects against new objects in one binary, 14976 generated cases per configuration: every stage output of the scalar, AVX2 and AVX-512 kernels is identical except the scale-0 contrast-masking stages, and the new scalar, AVX2 and AVX-512 kernels are identical to each other in every stage.
Tests
test_integer_adm_cm_threshold(rewritten): pins the clamp, and scores both patch pictures on the scalar path, the default dispatch and AVX2 alone. No score may exceed 1 and every level must return the scalar's bits. On master it fails with64x64 cpumask 4294967295 frame 0 integer_adm_scale0: 1.0829225419556654.test_integer_adm_simd(extended):adm_cm_avx2()andadm_cm_avx512()against the scalar kernels on hand-built bands, with thresholds of either sign in every column and threshold products that leave int32. Sixteen planted defects each fail it, among them upstream's own vector form, which differs from its scalar for a negative threshold.test_gpu_adm_tiny_frames(extended): the patch content on the CUDA, HIP and SYCL twins.adm_avx512.c:2291:17: runtime error: left shift of negative value -15176(andadm_avx2.c:2687:17with--cpumask 48); the branch reports nothing at--cpumask0, 48 and 4294967295. The thirteen ADM unit tests pass underUBSAN_OPTIONS=halt_on_error=1.test_cuda_adm_cm_register_pressure(no stack or local spill, at most 208 registers, as on master).Performance
Old and new objects in one benchmark, alternated on the same 1920x1080 frames (Ryzen 9 9950X3D, gcc 16.2.1,
-O3, minimum of 4 to 6 runs). The same benchmark with the old source in both slots gives 1.000.The AVX-512 pipeline as a whole runs 0% to 5% slower than before the split at an equal instruction count. I could not find the cause; it is filed as
T-ADM-AVX512-SPLIT-STAGE-TIME-2026-10-01. An end-to-endvmaf --feature admrun on BBB 3840x2160 does not resolve differences of this size on this host while other jobs run.Checklist
integer_adm_kernels.hcarries the Netflix + Lusoris BSD+Patent header of the file its code came from.docs/adr/_index_fragments/1402-adm-cm-centre-tap-int32.mdand the slug is appended to_order.txt.Bug-status hygiene
docs/state.mdupdated:T-ADM-CM-CENTRE-TAP-WRAP-ABOVE-ONE-2026-10-01andT-ADM-CM-X86-TAIL-NEGATIVE-THRESHOLD-SHIFT-2026-10-01moved to "Recently closed";T-ADM-DECOUPLE-X86-FRACTIONAL-GAIN-ROUNDING-2026-10-01andT-ADM-AVX512-SPLIT-STAGE-TIME-2026-10-01opened; the Netflix/vmaf PR 1602 row under "Confirmed not-affected" rewritten.Netflix golden-data gate
assertAlmostEqual(...)score in the Netflix golden Python tests. The gate was run before any twin was changed (scalar prototype, assembly disabled: 271 passed, 12 skipped, same outcome per test as master) and again on the finished change (271 passed, 12 skipped).Deep-dive deliverables
docs/research/1402-adm-cm-centre-tap-int32.md.## Alternatives consideredindocs/adr/1402-adm-cm-centre-tap-int32.md.AGENTS.mdinvariant note —core/src/feature/AGENTS.md,core/src/feature/x86/AGENTS.md,core/src/feature/sycl/AGENTS.md, and the index entry indocs/development/rebase-sensitive-invariants.md.changelog.d/fixed/adm-cm-centre-tap-int32.md.docs/rebase-notes.md, entry "fix/adm-cm-centre-tap-wrap": which upstream hunks to refuse when PR 1602 merges, and the function map of the split x86 files.Reproducer
Known follow-ups
integer_adm.metalis changed in source only. Its centre tap was already 32 bits wide; the excess now goes through an MSL twin ofadm_cm_excess_s0().test_metal_integer_adm_parityneeds an Apple device.T-ADM-DECOUPLE-X86-FRACTIONAL-GAIN-ROUNDING-2026-10-01(older than this change): with a non-integeradm_enhn_gain_limitthe AVX2 and AVX-512 decouple kernels round where the scalar truncates; up to 1.2e-6 oninteger_adm_scale0for a gain of 1.2 on the Netflix pair. Integer gains (the default 100, the 1.0 of the NEG models) are bit-identical.T-ADM-AVX512-SPLIT-STAGE-TIME-2026-10-01: see Performance..standards-baseline.jsonis not re-recorded. The audit passes (138 active violations within 182 baselined limit); this change removes 24 baselined findings, which leaves stale entries. Re-recording here would conflict with every other open pull request that does the same, so it is left for one re-record on master.integer_aimis 3.18 on the 64x64 patch picture wherefloat_adm'saimis 1, before and after this change.vmaf --backend hip --feature adm_hip --threads 4exits withproblem flushing contexton the gfx1036, on master and on this branch alike.