Skip to content

[SYCL][TEST]Fix half normalize tolerance in marray_geometric - #22516

Open
vishwajithupendra wants to merge 2 commits into
intel:syclfrom
vishwajithupendra:marray_geometric_half_normalize_tolerance
Open

[SYCL][TEST]Fix half normalize tolerance in marray_geometric#22516
vishwajithupendra wants to merge 2 commits into
intel:syclfrom
vishwajithupendra:marray_geometric_half_normalize_tolerance

Conversation

@vishwajithupendra

Copy link
Copy Markdown
Contributor

Use 1e-3 absolute delta for all half normalize calls, consistent with other half geometric tests (length, distance).

The normalize operation chains three half-precision rounding steps (dot product -> rsqrt -> per-component multiply), each introducing up to 1 ULP of error. At the output values (~0.18 to ~0.89), 1 ULP in half precision is already in the range ~1.7e-4 to ~6.6e-4, so tolerances of 1e-4 or tighter are too close to or below 1 ULP and risk false failures. 1e-3 is the minimum round tolerance that sits comfortably above the 1 ULP range at these values and covers the rounding differences observed on Adreno GPU.

Use 1e-3 absolute delta for all half normalize calls, consistent with
other half geometric tests (length, distance).

The normalize operation chains three half-precision rounding steps
(dot product -> rsqrt -> per-component multiply), each introducing
up to 1 ULP of error. At the output values (~0.18 to ~0.89), 1 ULP
in half precision is already in the range ~1.7e-4 to ~6.6e-4, so
tolerances of 1e-4 or tighter are too close to or below 1 ULP and
risk false failures. 1e-3 is the minimum round tolerance that sits
comfortably above the 1 ULP range at these values and covers the
rounding differences observed on Adreno GPU.
@vishwajithupendra
vishwajithupendra requested a review from a team as a code owner July 2, 2026 05:56
@hdelan

hdelan commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

Ping @uditagarwal97 @bader can we please run the CI and get a review on this? Thanks

@bader

bader commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

@vishwajithupendra, @hdelan, I started the CI.
Can we switch from checking absolute values to ULPs?

@intel/llvm-reviewers-runtime, please, review.

@VerenaBeckham

Copy link
Copy Markdown
Contributor

@vishwajithupendra, @hdelan, I started the CI. Can we switch from checking absolute values to ULPs?

@intel/llvm-reviewers-runtime, please, review.

Aha, we actually have #22867 as well, which makes a start on that. Only for this one it seemed to be the most minimal change to stick to absolute values and we weren't sure what ULP values would actually be reasonable, since SYCL doesn't dictate anything and the different backends will provide different accuracy.

@bader

bader commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

Only for this one it seemed to be the most minimal change to stick to absolute values and we weren't sure what ULP values would actually be reasonable, since SYCL doesn't dictate anything and the different backends will provide different accuracy.

Following this logic checking accuracies in general doesn't make sense for SYCL tests. ;)

From the practical POV, we run SYCL tests on limited number of back-ends. I suppose current test is adjusted for values returned by Intel's OpenCL/L0 implementation rather than to satisfy OpenCL spec requirements.

We could adjust the test to match specific SYCL backend accuracy requirements, but this is a separate effort beyond the current fix.

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.

4 participants