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

Move to better test infrastructure #690

Closed
jdemel opened this issue Nov 4, 2023 · 3 comments · Fixed by #767
Closed

Move to better test infrastructure #690

jdemel opened this issue Nov 4, 2023 · 3 comments · Fixed by #767
Labels
Enhancement new kernel entirely or for some specific ARCH Future Future Work

Comments

@jdemel
Copy link
Contributor

jdemel commented Nov 4, 2023

In a lot of issues and PRs we discuss problems with our current tests.

We need to discuss a way forward to improve this situation.

One option would be to introduce gtest. We would write specific tests for some kernels first and adopt an approach where we slowly move to the new system.

@jdemel jdemel added Enhancement new kernel entirely or for some specific ARCH Future Future Work labels Nov 4, 2023
@jdemel
Copy link
Contributor Author

jdemel commented Nov 5, 2023

I did some tests with gtest:
https://github.com/jdemel/volk/tree/newtest

At the moment, there are quite a few areas where this can be improved.

  1. Integration into ctest
  2. Output prints should go into the log instead of the default output
  3. Possible copypasta code should be reduced.
    Thus, this implementation is a proof of concept and open for discussion.

@argilo
Copy link
Member

argilo commented Nov 5, 2023

I don't have an opinion on which test framework to use, but I'll list out some things that could be improved by moving away from one-size-fits-all testing:

@argilo
Copy link
Member

argilo commented Dec 8, 2023

Another problematic case:

  • volk_32f_s32f_32f_fm_detect_32f and volk_32f_s32f_s32f_mod_range_32f both involve phase angle calculations, and tolerance checks fail if the angles being compared are -pi+epsilon and +pi-epsilon. (The difference in angle is very small, but the difference in absolute value is large.)

@jdemel jdemel linked a pull request Aug 19, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement new kernel entirely or for some specific ARCH Future Future Work
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants