Repository navigation
Conversation
lusoris
force-pushed
the
fix/cuda-adm-cm-scale0-border
branch
from
October 2, 2026 05:24
730baeb to
4d5bebb
Compare
… sums once per row The CPU (adm_cm(), i4_adm_cm()) sums a whole row of cubed contrast masking values and applies (accum_inner + add_shift_inner_accum) >> shift_inner_accum once. The CUDA kernels apply that shift to the partial sum of every warp before the atomic add: the fused scale 0 kernel to each 32x8 tile, adm_cm_reduce_line_kernel to each warp of 128 columns. Rounding is not additive, so the accumulators drift from the CPU's, by O(1) per extra warp. In the near-zero accumulators of smooth content that is a large relative error: integer_adm_scale0 of a 38x38 structured frame is off by 0.31. adm_cm_line_kernel stores its masked values per pixel in the accumulator buffer, as i4_adm_cm_line_kernel already does, and adm_cm_reduce_line_kernel (one block per band and row, reduced in shared memory) cubes them and applies the shift once per row. The scale 0 path launches it after the fused kernel, so the reduction constants of the scale 0 bands, which the kernel already carried, are used. The scale 0 rounding constant of the horizontal and vertical bands was computed on the host as 1 << (shift - 1) with a shift of 0 for frame widths 17 to 32. That is 2^31 on x86. It is now computed in the reduction kernel, with no rounding term for a shift of 0. Add test_cuda_adm_cm_row_rounding, which compares integer_adm_scale0 of adm_cuda with the scalar CPU adm on random and structured frames of 38x38 to 200x120 at 8 and 10 bits. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…e border
adm_cm_line_kernel() reads the neighbours of a sample at positions
abs(x - 1), x and x + 1, and subtracts max(0, 2 * (x - w) + 1) from the
last one (rows likewise). The correction tests the sample's own position
against the band, not the neighbour's, so it never applies: x is inside
the band. For a band of 14 samples or fewer in a dimension, a frame of 17
to 28 pixels, the contrast masking runs to the band's last column and row,
and the neighbour at x + 1 is column w (row h), outside the band. The CPU
replicates the last sample: {w - 2, w - 1, w - 1}.
Clamp both neighbours with min(position, n - 1). The rows of a thread that
lie below the band are masked off and unchanged, but they no longer read
outside it.
Add test_cuda_adm_cm_scale0_border, which compares integer_adm_scale0 of
adm_cuda with the scalar CPU adm on frames of 17 to 28 pixels at 8 and 10
bits.
Applies on top of the row rounding change: that one removes the rounding
error that otherwise hides this one.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
lusoris
force-pushed
the
fix/cuda-adm-cm-scale0-border
branch
from
October 2, 2026 18:45
4d5bebb to
d3df91e
Compare
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
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.
integer_adm_scale0ofadm_cudadiffers from the CPU by up to 7.8e-2 on frames of 17 to 28 pixels once the row rounding is fixed (previous change in the series), and the kernel reads outside the band at those sizes.This change applies on top of the row rounding change: that one removes the rounding error that otherwise hides this one, and the branch is stacked on it.
Cause
adm_cm_line_kernel()(libvmaf/src/feature/cuda/integer_adm/adm_cm.cu) reads the neighbours of a sample at the positionsabs(x - 1),xandx + 1, and subtractsmax(0, 2 * (x - w) + 1)from the last one; rows likewise, withy, the first row of the thread. The correction tests the sample's own position against the band, not the neighbour's, so it never applies:xis inside the band. For a scale 0 band of 14 samples or fewer in a dimension (a frame of 17 to 28 pixels)leftandtopare 0, the contrast masking runs to the band's last column and row, and the neighbour atx + 1is columnw(rowh), outside the band. The CPU replicates the last sample there,{w - 2, w - 1, w - 1}(ADM_CM_THRESH_S_*,integer_adm.c).The rows of a thread that lie below the band are masked off, so they do not change the score, but they read past the band as well.
Reproducer
Master
8e7a1ac4e,-Denable_cuda=true, RTX 4090, C API program printing%.17g(the new test makes the same comparison). A structured 20x20 8-bit frame (generator of the test),integer_adm_scale0:integer_adm_scale0adm_cuda, masteradm_cuda, row rounding change onlyadm_cuda, this branch on top of itSweep of 212 configurations (17x17 to 64x64 and fifteen other sizes up to 320x180, 8 and 10 bits, random and structured content, two frames), largest absolute difference of
integer_adm_scale0against the scalar CPU over the configurations of the size group:Rows (size, bit depth) with a scale 0 difference: 30 with the row rounding change alone, 2 with this change on top (55x55 and 320x180 at 8 bits, the angle test of the decouple stage, a separate change).
Scale 0 of the CPU is the same on master and with the fixes of #1599, #1600, #1602 and #1633 at every size of the sweep (checked on all 212 configurations), so the test needs none of them.
Fix
Both neighbours are clamped with
min(position, n - 1):pos_x[0] = min(abs(x - 1), w - 1),pos_x[2] = min(x + 1, w - 1), and the same for the rows. For a sample inside the band that is the old position at the top and left (mirror), and the replicated last sample at the bottom and right.Tests
test_cuda_adm_cm_scale0_border(new,libvmaf/test/, built withenable_cuda; it reports a skip when no CUDA device can be opened) comparesinteger_adm_scale0ofadm_cudawith the scalar CPUadmon 17x17, 20x20, 24x24, 28x28, 17x64 and 64x17 at 8 and 10 bits, random and structured content, with a tolerance of 1e-9. Unpatched master fails it (17x17, 8 bit, random: 1.8e-3), the row rounding change alone fails it too (1.81e-3 on the same case), this branch passes it.Validation
Master
8e7a1ac4e, 2026-10-01. x86-64 Linux, GCC 16.2.1, nvcc 13.4, RTX 4090.Rebased on master
9e48141b(2026-10-02). On that base, with-Denable_cuda=true -Denable_float=true -Denable_checkasm=true,meson testgives 29 of 30 (the failure istest_cuda_pic_preallocation, which also fails on unpatched9e48141b, 27 of 28 there), and the CPU-Db_sanitize=address,undefined -Db_lto=falsebuild gives 22 of 25 on master and here (test_predictandtest_pic_preallocationabort under LeakSanitizer andcheckasmaborts on a heap-buffer-overflow inadm_dwt2_16(integer_adm.c:2603); all three also abort on unpatched9e48141b). The CLI output of the three Netflix pairs (--gpumask 0,vmaf_v0.6.1) against unpatched9e48141b: identical on all three pairs (src01 76.668905, checkerboard 1 px 35.068667, 10 px 7.985899). The sweeps and the other measurements below were taken on8e7a1ac4eand not repeated; the files and x86 code paths they depend on are unchanged since (the one upstream change in between is an arm64-only ADM kernel).adm_cudaat%.17g(integer_adm2, scales 0 to 3, numerators and denominators):src01576x324 8 bit (48 frames) and 10 bit (3 frames), checkerboard 1 px and 10 px 1920x1080 (3 frames each), KristenAndSara 1280x720,akiyo352x288,sparks480x270 10 bit (5 frames): identical to both. On the 18x22akiyoclip the largest scale 0 difference to the CPU goes from 2.4e-2 (row rounding change alone) to 0 (all five branches merged; scales 1 to 3 close in the border change). No golden assertion changes.compute-sanitizer --tool memcheckreports no error for master and for the merge of the five branches at 24x24 and 20x36: the rows and columns past the band are inside the allocation.libvmaf/test/meson.buildconflicts textually. cuda: keep kernel parameter structs and accumulators out of local memory #1595 and cuda: build the kernels with clang without the CUDA toolkit headers #1596 changeadm_cm.culines that the row rounding change removes.meson teston the merge of the five branches: 28 of 29 pass.test_cuda_pic_preallocationaborts with SIGSEGV intest_cuda_picture_preallocation_method_host_pinned; it does the same on unpatched master.The workflow run on this PR needs a maintainer's approval.