Add functionality to print hashes and computed errors in tests #778
Conversation
rocSOLVER's tests currently output no useful information for passing tests, which makes it hard to ascertain when new code adversely affects the accuracy of methods while also hindering the investigation of bugs that may affect determinism. This commit adds code to print the computed error in each test and hashing functions to allow printing hashes of input and output matrices, when desired.
jmachado-amd
requested review from
jzuniga-amd,
tfalders,
cgmb,
qjojo and
EdDAzevedo
as code owners
jmachado-amd
added
the
noOptimizations
There was a problem hiding this comment.
The std::hash may be implementation dependent. One may consider another hash function such as the following (found using edge browser) to make this more portable across OS and compiler versions. Just a suggestion.
unsigned hash(const char* s, unsigned salt) {
unsigned h = salt;
while (*s)
h = h * 101 + static_cast<unsigned>(*s++);
return h;
}
| #define ANSI_CODE_GTEST_GREEN "\033[0;32m" | ||
| #define ANSI_CODE_NORMAL_TERM "\033[0;0m" |
There was a problem hiding this comment.
If we're going to print with colour, we should follow Google Test's lead by obeying the gtest_color option and using TTY detection for the default. I'd suggest printing without colour for now, as that is a bit of a distraction from the main purpose of this change.
There was a problem hiding this comment.
Also, the fmtlib that we use in rocsolver has a color API. https://fmt.dev/11.0/api/#color-api
| #define ANSI_CODE_NORMAL_TERM "\033[0;0m" | ||
| // Macro ROCSOLVER_GTEST_MSG_PRINTER is also used to print hashes in tests | ||
| #define ROCSOLVER_GTEST_MSG_PRINTER \ | ||
| std::cout << ANSI_CODE_GTEST_GREEN << "[ ] " << ANSI_CODE_NORMAL_TERM |
There was a problem hiding this comment.
Use fmt. We added it as a dependency to do atomic prints and avoid the iostreams interfaces where the printing of a line may be split into multiple function calls. It's not that important at the moment, but it's an invariant we inherited from rocfft / rocblas and I want to retain it in case we do ever run tests in parallel.
| ROCSOLVER_GTEST_MSG_PRINTER \ | ||
| << "Computed error: " << ROCSOLVER_STRNGFY(max_error) << " / " \ | ||
| << ROCSOLVER_STRNGFY(((tol)*get_epsilon<T>())) << " = " \ | ||
| << std::setprecision(max_precision) \ | ||
| << ((max_error_ >= 0.) && (tol_ > 0.) ? max_error_ / tol_ : -1.) \ | ||
| << std::setprecision(default_precision) << std::endl \ | ||
| << std::flush; \ |
There was a problem hiding this comment.
This would be so much nicer using the fmtlib interface.
| @@ -263,6 +263,8 @@ void sygvx_hegvx_initData(const rocblas_handle handle, | |||
| // for testing purposes, we start with a reduced matrix M for the standard equivalent problem | |||
| // with spectrum in a desired range (-20, 20). Then we construct the generalized pair | |||
| // (A, B) from there. | |||
|
|
|||
| [[maybe_unused]] volatile auto mptr = memset(hB[b], 0, n * ldb * sizeof(T)); | |||
There was a problem hiding this comment.
I assume the volatile auto mptr is an attempt to ensure that memset is not optimized out, but I'm not sure that's guaranteed. Why is this needed?
| ROCSOLVER_GTEST_MSG_PRINTER << "Input matrix hash: " << input_hash << std::endl << std::flush; | ||
| ROCSOLVER_GTEST_MSG_PRINTER << "Rocsolver eigenvalues hash: " << rocsolver_eigenvalues_hash | ||
| << std::endl | ||
| << std::flush; |
There was a problem hiding this comment.
std::endl is defined as a newline and a flush, so this is redundant. Although, again, I would suggest using fmt rather than iostreams.
rocSOLVER's tests currently output no useful information for passing tests, which makes it hard to ascertain when new code adversely affects the accuracy of methods while also hindering the investigation of bugs that may affect determinism.
This PR adds code to print the computed error in each test and hashing functions to allow printing hashes of input and output matrices, when desired.