Skip to content

Comments

[SYCL][ESIMD]Adjust the test to pass when used with fast-math option#9723

Closed
fineg74 wants to merge 1 commit intointel:syclfrom
fineg74:fastmathFix1
Closed

[SYCL][ESIMD]Adjust the test to pass when used with fast-math option#9723
fineg74 wants to merge 1 commit intointel:syclfrom
fineg74:fastmathFix1

Conversation

@fineg74
Copy link
Contributor

@fineg74 fineg74 commented Jun 2, 2023

No description provided.

@fineg74 fineg74 requested a review from a team as a code owner June 2, 2023 18:01
@fineg74 fineg74 temporarily deployed to aws June 2, 2023 18:11 — with GitHub Actions Inactive
@fineg74 fineg74 temporarily deployed to aws June 2, 2023 18:52 — with GitHub Actions Inactive
float std_result_atan = std::atan2(input_x, input_y);

if (std::fabs(std_result_atan - vector_output_atan2[iy * size + ix]) >
0.0001f) {
Copy link
Contributor

@v-klochkov v-klochkov Jun 2, 2023

Choose a reason for hiding this comment

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

  1. The test without fast-math passes with the difference 0.0001f. With the proposed change the test compiled without fast-math might eventually start giving difference 3.0f (which most-likely is due to some error), and such error would not be shown by the updated test.
    Maybe this would work better:
#ifdef __FAST_MATH__ <use one tolerance> #else <use 0.0001f tolerance> #endif
  1. 3.2f seems like quite a big number. It may be Ok for fast-math and atan depending on the input values, I don't know.
    Are you sure the device code computes result correctly taken into account such a big difference?

@github-actions
Copy link
Contributor

github-actions bot commented Feb 1, 2024

This pull request is stale because it has been open 180 days with no activity. Remove stale label or comment or this will be automatically closed in 30 days.

@github-actions github-actions bot added the Stale label Feb 1, 2024
@fineg74 fineg74 closed this Mar 28, 2024
@fineg74 fineg74 deleted the fastmathFix1 branch March 28, 2024 02:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants