Skip to content

tests: allow an ulp between the HDR Bayer and general CFA samplers - #22816

Merged
TurboGit merged 1 commit into
darktable-org:masterfrom
nicknezis:hdr-alignment-sampler-test-tolerance
Oct 28, 2026
Merged

TurboGit merged 1 commit into
darktable-org:masterfrom
nicknezis:hdr-alignment-sampler-test-tolerance

Conversation

@nicknezis

Copy link
Copy Markdown
Contributor

Summary

test_hdr_alignment_internal / test_bayer_sampler_bit_exact asserts that
the Bayer fast sampler and the general CFA sampler return bit-identical
floats. Both sum the same taps in the same order, but release builds use
-ffast-math -march=native, so the compiler can fuse or reorder the two
loops differently depending on the CPU. That is why
Linux_ubuntu:26.04_LLVM22_Release_tests fails intermittently since
ctest was enabled in all Linux jobs (#22799): 3 of the last 5 master runs.
On an AVX-512 machine with clang 18, 11 of 153456 samples differ, by at
most 1.0e-07 relative. The test now compares within 1e-6 relative, and
the comments no longer claim bit-for-bit equality. No production code
changes.

Checklist

  • I have read CONTRIBUTING.md and the coding style.
  • I have not merged master into the topic branch.
  • The pull request is one logical change, and every commit compiles on its own.
  • I ran the relevant tests: full ctest (Release, clang 18, OpenCV 4.6), 30/30 pass.
  • New user-visible strings use _() (none added).
  • A RELEASE_NOTES.md entry was added: not needed, test-only change.

Test instructions

Configure with -DBUILD_TESTING=ON and OpenCV, Release build, then
ctest -R test_hdr_alignment_internal. On master it fails on an AVX-512
x86 box with clang 18; with this change it passes. Not tried with
clang 22 or GCC 16.

AI assistance

Written with Claude Code (Anthropic), which reproduced the failure and
wrote the change; I reviewed it.

test_bayer_sampler_bit_exact asserted that _sample_bayer_same_color()
and _sample_cfa_same_color() return bit-identical floats. Both sum the
same taps in the same order, but release builds use -ffast-math and
-march=native, so the compiler is free to fuse or reorder the two loops
differently, and whether it does depends on the CPU. With clang on an
AVX-512 machine 11 of the 153456 samples differ, by at most 1.0e-07
relative; the test then fails, which is why the LLVM job of the Linux
CI fails on some runners and passes on others.

Compare within 1e-6 relative instead, and stop claiming bit-for-bit
equality in the comments.
@kofa73

kofa73 commented Oct 28, 2026

Copy link
Copy Markdown
Collaborator

I had Codex (GPT-6.1 Sol) investigate this before I saw the PR. It found the same issue, reproduced it locally by forcing different CPU optimisations with Clang 22: Skylake target failed 10/10, Haswell passed 10/10. It then found the PR, tried the proposed solution and verified that it is working.

@TurboGit TurboGit added this to the 5.8 milestone Oct 28, 2026

@TurboGit TurboGit left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@TurboGit TurboGit added the scope: codebase making darktable source code easier to manage label Oct 28, 2026
@TurboGit
TurboGit merged commit f9dbc77 into darktable-org:master Oct 28, 2026
10 checks passed
@nicknezis
nicknezis deleted the hdr-alignment-sampler-test-tolerance branch October 28, 2026 18:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

scope: codebase making darktable source code easier to manage

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants