Skip to content

[syclcompat][E2E] Skip case in math_vectorized_isgreater_test if fp16 unsupported #14596

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Conversation

steffenlarsen
Copy link
Contributor

The second test case in sycl/test-e2e/syclcompat/math/math_vectorized_isgreater_test.cpp uses vector of sycl::half, which requires devices to support aspect::fp16. This commit makes the test skip this case if the device does not support the aspect.

… unsupported

The second test case in
sycl/test-e2e/syclcompat/math/math_vectorized_isgreater_test.cpp uses
vector of sycl::half, which requires devices to support aspect::fp16.
This commit makes the test skip this case if the device does not
support the aspect.

Signed-off-by: Larsen, Steffen <[email protected]>
@steffenlarsen steffenlarsen requested a review from a team as a code owner July 17, 2024 10:25
@Alcpz
Copy link
Contributor

Alcpz commented Jul 17, 2024

@steffenlarsen #14594 addressed this issue (though it was really disabling the test for that case). We are happy with both solutions, though yours seems better.

@steffenlarsen
Copy link
Contributor Author

@steffenlarsen #14594 addressed this issue (though it was really disabling the test for that case). We are happy with both solutions, though yours seems better.

My bad, @Alcpz ! Alternatively, we can also split the test, making the aspect requirement in the one using half. The reason I went for a skip of only part was to still cover the supported case, but we could do that with a split as well. Either way, we can probably make it a follow-up if that is the way we want to go.

@Alcpz
Copy link
Contributor

Alcpz commented Jul 17, 2024

No problem, Thanks for the contribution! I think this skip is good, it's a relatively simple test and creating a whole new test case for the split seems too much.

@steffenlarsen steffenlarsen merged commit fe9e0ba into intel:sycl Jul 17, 2024
15 checks passed
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.

2 participants