Skip to content

fix(sycl): mirror the integer-ADM contrast-masking near edge, and extend large-fixture parity to SYCL - #1366

Closed
lusoris wants to merge 3 commits into
fix/gpu-parity-adm-edge-and-ssimulacra2-fmafrom
test/gpu-parity-large-fixture-other-backends
Closed

lusoris wants to merge 3 commits into
fix/gpu-parity-adm-edge-and-ssimulacra2-fmafrom
test/gpu-parity-large-fixture-other-backends

Conversation

@lusoris

@lusoris lusoris commented Sep 6, 2026

Copy link
Copy Markdown
Contributor

Summary

Stacked on #1363 (base is that branch; GitHub retargets to master when it merges).

Extends the ADR-1206 large-fixture parity variants from CUDA to SYCL, and fixes the real bug that immediately found.

ADR-1210 — test_sycl_adm_parity was failing on the Arc A380, on its shipped fixture:

integer_adm_scale3_csf_2_...: cpu=0.58175555 sycl=0.58191226 delta=1.57e-04 tol=1.00e-04

The CPU contrast-masking rule is asymmetric — near edge mirrors to index 1, far edge clamps to the last index (integer_adm.c:1009-1012). The SYCL twin clamped both, reading row/column 0 twice and dropping the mirrored sample. It only diverges once a scale's ADM border crop (int)(dim * 0.1 - 0.5) collapses to 0 — band dimensions ≤ 14 — and scale 3 of the 256x144 fixture is a 16x9 band, exactly that regime.

CUDA (adm_cm.cu), HIP (adm_cm.hip) and Metal (iadm_clampx) all already carry this fix from ADR-1167 / PR #1224. SYCL was the only twin that never received it. Green in CI only because the test skips with no SYCL device — every hosted runner.

suite (Arc A380) before after
SYCL parity, shipped fixtures 17 pass / 1 fail / 2 skip 18 pass / 0 fail / 2 skip
SYCL parity, 960x540 variants — 16 pass / 0 fail / 3 skip

Two variants are handled explicitly rather than silently:

  • float_ssim_sycl is a documented v1 scale=1-only extractor; its large variant records that contract as a skip, and still fails loudly if the twin ever stops refusing and starts returning a scale=1 score at a decimating resolution.
  • test_sycl_motion_add_uv_parity is not registered. It compares CPU float float_motion against fixed-point motion_sycl, so its 2e-4 tolerance is a quantisation budget calibrated for one fixture, not a bit-exactness bound; at 960x540 it lands at 2.30e-04. Raising the tolerance to make it pass would be exactly the test-weakening the fork forbids, so it is tracked as T-SYCL-MOTION-ADD-UV-TOLERANCE-RESOLUTION-2026-09-06 instead.

HIP and Metal large variants are deliberately not registered: Metal cannot be verified on this workstation at all, and shipping test registrations that have never been run is how a lane goes red for reasons nobody has looked at.

Type

  • fix — bug fix
  • test — test-only
  • sycl / cuda / simd — backend-specific

Checklist

  • Commits follow Conventional Commits.
  • make format && make lint is green locally (pre-commit run --files clean).
  • Unit tests pass: meson test -C build — SYCL suites on an Arc A380: 18/0/2 shipped, 16/0/3 large.
  • If I touched any SIMD/GPU code path, I ran /cross-backend-diff and the worst ULP is ≤ 2 — equivalent done directly: the CPU-vs-SYCL integer_adm_scale3_csf_2 delta goes from 1.57e-04 to inside the 1e-4 gate, and every other SYCL parity feature passes at both fixtures.
  • If I touched a feature extractor with SIMD/GPU twins, I either updated every twin or listed the gap — CUDA, HIP and Metal already had this fix; only SYCL needed it, verified by reading all four.
  • If I added a new .c / .h — no new source files.
  • If this is a breaking change — not breaking. SYCL integer_adm scores move at coarse-band sizes ≤ 14 px, towards the CPU reference; no fork-added snapshot covers that.
  • If this PR adds an ADR, the row lives in docs/adr/_index_fragments/ — index in sync.

Bug-status hygiene (ADR-0165)

  • docs/state.md updated — T-SYCL-INTEGER-ADM-CM-NEAR-EDGE-2026-09-06 closed, T-SYCL-MOTION-ADD-UV-TOLERANCE-RESOLUTION-2026-09-06 opened.

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 — none does. The golden gate is CPU-only; this changes a SYCL kernel.

Deep-dive deliverables (ADR-0108)

  • Research digest — no digest needed: the method is already written up in docs/research/2032-gpu-parity-resolution-blind-spot.md (added by fix(gpu): clamp the ADM contrast-masking far edge and finish the ssimulacra2 FMA unification #1363) and this is a direct application of it; ADR-1210 carries the specific evidence.
  • Decision matrix — ## Alternatives considered in ADR-1210 (including why loosening the 1e-4 tolerance was rejected) and the extended ADR-1206.
  • AGENTS.md invariant note — added to core/test/AGENTS.md.
  • Reproducer / smoke-test command — below.
  • CHANGELOG fragment — changelog.d/fixed/sycl-integer-adm-cm-near-edge.md and changelog.d/fixed/sycl-parity-large-fixture-variants.md.
  • Rebase note — docs/rebase-notes.md, "ADR-1206 (SYCL) — which parity tests get a large-fixture variant".

Reproducer

Needs a host with an Intel GPU — without one the tests skip and report green, which is how this survived.

CC=icx CXX=icpx meson setup build core -Denable_sycl=true -Denable_cuda=false
ninja -C build
meson test -C build --suite sycl

On the parent commit test_sycl_adm_parity fails with delta=1.57e-04 tol=1.00e-04; on this branch the suite is 18/0.

Known follow-ups

  • HIP and Metal large-fixture variants (unverifiable here; Metal has no hardware on this workstation).
  • T-SYCL-MOTION-ADD-UV-TOLERANCE-RESOLUTION-2026-09-06 — needs a derived, resolution-aware fixed-point-vs-float bound, not a bumped constant.
  • The SYCL Parity (Arc A380) lane builds --buildtype=release; this was found in a debug build. The two configurations should be reconciled so the lane sees what a developer sees.

🤖 Generated with Claude Code

lusoris and others added 3 commits September 6, 2026 19:25
…e (ADR-1206)

Extends the ADR-1206 large-fixture variants from CUDA to SYCL. Every SYCL
parity test pinned one small fixture, so the shared SSIM/MS-SSIM auto-scale
`max(1, round(min(w, h) / 256))` never left 1 and the ADM border crop never
left 0 — the region where ADR-1202, ADR-1204 and the float_ssim scale=1-only
limitation all hid.

Verified on an Arc A380: 16 large variants pass, 3 skip. The sweep found two
things. `float_ssim_sycl` is a documented v1 scale=1-only extractor, so its
large variant now records that contract as a skip (and still fails loudly if
the twin ever stops refusing and starts returning a scale=1 score at a
decimating resolution). `test_sycl_motion_add_uv_parity` is not registered:
it compares the CPU float `float_motion` against the fixed-point `motion_sycl`,
so its 2e-4 tolerance is a quantisation budget calibrated for one fixture
rather than a bit-exactness bound, and at 960x540 it lands at 2.30e-04 — over
a bound never derived for that resolution. Tracked as
T-SYCL-MOTION-ADD-UV-TOLERANCE-RESOLUTION-2026-09-06 rather than papered over
by raising the tolerance.

HIP and Metal are deliberately not registered: Metal cannot be verified on
this workstation at all, and shipping test registrations that have never been
run is how a lane goes red for reasons nobody has looked at.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
test_sycl_adm_parity was failing on the Arc A380 on its shipped fixture:
integer_adm_scale3_csf_2 cpu=0.58175555 sycl=0.58191226, delta 1.57e-04
against the places=4 gate. Green in CI only because the test skips cleanly
with no SYCL device, which is every hosted runner.

The CPU contrast-masking rule is asymmetric — the near edge mirrors to index 1,
the far edge clamps to the last index (integer_adm.c:1009-1012). The SYCL twin
clamped both, so row/column 0 was read twice and the mirrored sample dropped.
It only diverges once a scale's border crop (int)(dim * 0.1 - 0.5) collapses
to 0 — band dimensions <= 14 — and scale 3 of the 256x144 fixture is a 16x9
band, exactly that regime.

CUDA, HIP and Metal all already carry this fix (ADR-1167 / PR #1224). SYCL was
the only twin that never received it.

Verified on an Arc A380: test_sycl_adm_parity 6/6, shipped SYCL parity suite
17 pass/1 fail -> 18 pass/0 fail, large-fixture suite 16 pass/0 fail.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@lusoris
lusoris deleted the branch fix/gpu-parity-adm-edge-and-ssimulacra2-fma September 6, 2026 17:50
@lusoris lusoris closed this Sep 6, 2026
lusoris added a commit that referenced this pull request Sep 6, 2026
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
lusoris added a commit that referenced this pull request Sep 7, 2026
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
lusoris added a commit that referenced this pull request Sep 7, 2026
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
lusoris added a commit that referenced this pull request Sep 7, 2026
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
lusoris added a commit that referenced this pull request Sep 7, 2026
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
lusoris added a commit that referenced this pull request Sep 7, 2026
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
lusoris added a commit that referenced this pull request Sep 7, 2026
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
lusoris added a commit that referenced this pull request Sep 15, 2026
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
lusoris added a commit that referenced this pull request Sep 15, 2026
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
lusoris added a commit that referenced this pull request Sep 15, 2026
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
lusoris added a commit that referenced this pull request Sep 15, 2026
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant