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

Fix the relative error computation (used in SpGEMM and Jacobi-fused SpGEMM tests) #781

Merged
merged 1 commit into from
Aug 5, 2020

Conversation

seheracer
Copy link
Contributor

@seheracer seheracer commented Aug 4, 2020

This PR fixes a minor bug in the relative error computation. With this change, the normalization is performed only when both of the entries have nonzero values.

This PR is expected to get rid of the failures in #780.

Spot check on white:

#######################################################
PASSED TESTS
#######################################################
cuda-10.1.105-Cuda_OpenMP-release build_time=639 run_time=137
cuda-10.1.105-Cuda_Serial-release build_time=613 run_time=183
cuda-9.2.88-Cuda_OpenMP-release build_time=673 run_time=157
cuda-9.2.88-Cuda_Serial-release build_time=649 run_time=203
gcc-6.4.0-OpenMP_Serial-release build_time=221 run_time=138
gcc-7.2.0-OpenMP-release build_time=151 run_time=45
gcc-7.2.0-OpenMP_Serial-release build_time=216 run_time=138
gcc-7.2.0-Serial-release build_time=139 run_time=54
#######################################################
FAILED TESTS
#######################################################
ibm-16.1.1-Serial-release (build failed)
#######################################################

This ibm failure is a configuration error, which is not relevant to my changes.

Spot check on kokkos-dev-2:

#######################################################
PASSED TESTS
#######################################################
clang-8.0-Cuda_OpenMP-release build_time=633 run_time=121
clang-8.0-Pthread_Serial-release build_time=204 run_time=104
clang-9.0.0-Pthread-release build_time=121 run_time=48
clang-9.0.0-Serial-release build_time=129 run_time=38
cuda-10.1-Cuda_OpenMP-release build_time=811 run_time=121
cuda-9.2-Cuda_Serial-release build_time=781 run_time=173
gcc-4.8.4-OpenMP-release build_time=115 run_time=40
gcc-7.3.0-OpenMP-release build_time=228 run_time=39
gcc-7.3.0-Pthread-release build_time=132 run_time=49
gcc-8.3.0-Serial-release build_time=142 run_time=40
gcc-9.1-OpenMP-release build_time=181 run_time=38
gcc-9.1-Serial-release build_time=166 run_time=40
intel-17.0.1-Serial-release build_time=252 run_time=42
intel-18.0.5-OpenMP-release build_time=356 run_time=40
intel-19.0.5-Pthread-release build_time=425 run_time=50


@seheracer seheracer requested a review from srajama1 August 4, 2020 21:58
@seheracer seheracer self-assigned this Aug 4, 2020

if(denominator > KATM::zero() )
val_diff = val_diff / denominator;
if(KAT::abs(view1(i)) > KATM::zero() && KAT::abs(view2(i)) > KATM::zero()) {
Copy link
Contributor

Choose a reason for hiding this comment

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

This looks fine, I am taking it that view1 stores test values and view2 stores reference values?
One small comment, it seems that eps_type should be the same as mag_type for proper comparison.
Also technically you should not need to check KAT::abs(view1(i)) > KATM::zero() but it should not hurt to do so.

Copy link
Contributor

@lucbv lucbv left a comment

Choose a reason for hiding this comment

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

This looks fine to me thanks for making the changes.

@ndellingwood ndellingwood merged commit 7dc70b0 into kokkos:develop Aug 5, 2020
@seheracer seheracer deleted the fixRelativeError branch August 5, 2020 18:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants