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

Decrease the range for signed 16-bit integer testing #706

Merged
merged 1 commit into from
Dec 1, 2023

Conversation

argilo
Copy link
Member

@argilo argilo commented Nov 11, 2023

Mitigates #676.

The qa_volk_16ic_x2_dot_prod_16ic test is now the most frequent source of CI failures which does not already have an open pull request. The volk_16ic_x2_dot_prod_16ic kernel was added in #77, and because it does not handle integer overflow consistently, the test suite was modified at that time to limit signed 16-bit integer testing to the range -7 .. +7. Unfortunately, this is not sufficient to prevent overflows in the qa_volk_16ic_x2_dot_prod_16ic.

It would be quite a bit of work to rewrite the volk_16ic_x2_dot_prod_16ic kernel to have consistent overflow handling between the protokernels, so I'm proposing to decrease the signed integer range slightly (to -6 .. +6). This does not remove test failures entirely, but reduces the failure rate by about 50x.

Copy link
Contributor

@jdemel jdemel left a comment

Choose a reason for hiding this comment

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

LGTM. Even though, this affects other kernels as well.

The saturation vs. overflow arithmetic that is mentioned in #676 is not resolved here. Thus, I leave that issue open.

@jdemel jdemel merged commit aa6ed1a into gnuradio:main Dec 1, 2023
33 checks passed
@argilo
Copy link
Member Author

argilo commented Dec 1, 2023

Even though, this affects other kernels as well.

That's correct; this will slightly reduce the effectiveness of testing for all kernels with 16i inputs, but I think it's better than having a flaky test which causes CI to be ignored. Hopefully we can come up with a proper fix for #676, after which the full range of 16i values can be tested.

@argilo argilo deleted the 16i-adjustment branch December 2, 2023 16:47
Alesha72003 pushed a commit to Alesha72003/volk that referenced this pull request May 15, 2024
Decrease the range for signed 16-bit integer testing
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