Skip to content
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

qa_volk_16ic_x2_dot_prod_16ic is flaky #676

Open
argilo opened this issue Oct 22, 2023 · 1 comment
Open

qa_volk_16ic_x2_dot_prod_16ic is flaky #676

argilo opened this issue Oct 22, 2023 · 1 comment
Labels

Comments

@argilo
Copy link
Member

argilo commented Oct 22, 2023

Example of a failed CI run: https://github.com/gnuradio/volk/actions/runs/6606089847/job/17941931107

Failures can be reproduced locally with:

ctest -R qa_volk_16ic_x2_dot_prod_16ic --output-on-failure --repeat until-fail:100000

I would guess that the failure is due to different arithmetic (modular vs. saturating) across the kernels.

The test logic intentionally uses very small integers to avoid overflow/saturation conditions, but this is not foolproof when the input vectors are long:

volk/lib/qa_utils.cc

Lines 75 to 77 in 42f57cd

case 2:
if (type.is_signed)
((int16_t*)data)[i] = (int16_t)((int16_t)scaled_rand % 8);

I think the kernels should be adjusted so they perform the same type of arithmetic. (Saturating arithmetic everywhere?)

Log:

18: RUN_VOLK_TESTS: volk_16ic_x2_dot_prod_16ic(131071,1)
18: generic completed in 0.304401 ms
18: a_sse2 completed in 0.0966 ms
18: u_sse2 completed in 0.0554 ms
18: u_avx2 completed in 0.0587 ms
18: a_avx2 completed in 0.0393 ms
18: offset 0 in1: -32533 in2: -32768 tolerance was: 0
18: volk_16ic_x2_dot_prod_16ic: fail on arch a_sse2
18: offset 0 in1: -32533 in2: -32768 tolerance was: 0
18: volk_16ic_x2_dot_prod_16ic: fail on arch u_sse2
18: offset 0 in1: -32533 in2: -32768 tolerance was: 0
18: volk_16ic_x2_dot_prod_16ic: fail on arch u_avx2
18: offset 0 in1: -32533 in2: -32768 tolerance was: 0
18: volk_16ic_x2_dot_prod_16ic: fail on arch a_avx2
@jdemel
Copy link
Contributor

jdemel commented Nov 4, 2023

I agree that saturation arithmetic should be applied everywhere. It'll cause less distortion in algorithms that use convolutions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants