Skip to content

Conversation

@lucamar
Copy link
Contributor

@lucamar lucamar commented Aug 9, 2019

After merging the pull request #889, we are getting the following error:

 MPI test on GPU with OpenACC using  8 tasks
 Result : FAIL
 Expected value sum(f3):  55. actual value: 110.

I think that there is a mistake in the fix submitted by @cosunae: the array that should be checked for the EXPECTED_CPP_STD_SUM is f2 and not f3, since only the first one is called as argument in call_cpp_std. What do you think @lxavier?

P.S.: I have replaced cudaThreadSynchronize() with cudaDeviceSynchronize() in compute_cuda.cu since the first one is deprecated.

@lucamar lucamar added the bug label Aug 9, 2019
@lucamar lucamar requested a review from ajocksch August 9, 2019 13:22
@lucamar lucamar self-assigned this Aug 9, 2019
Replace deprecated cudaThreadSynchronize() with cudaDeviceSynchronize()
@lxavier
Copy link
Contributor

lxavier commented Aug 9, 2019

it was in fact already failing before the fix, and we try to correct but obviously we did it wrong

write (*,*) "Result : FAIL"
write (*,*) "Expected value sum(f1): ", EXPECTED_CUDA_SUM, "actual value:", sum(f1)
else if (sum(f3) /= EXPECTED_CPP_STD_SUM) then
else if (sum(f2) /= EXPECTED_CPP_STD_SUM) then
Copy link
Contributor

Choose a reason for hiding this comment

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

no it should be f3, but the error is on the right side

Copy link
Contributor

Choose a reason for hiding this comment

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

like this:

else if (sum(f3) /= EXPECTED_CUDA_SUM) then

?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, basicall revertting the change from PR #888 only the file openacc_cuda_mpi_cppstd.F90 . I did that with Carlos, because the code was failling. But probably because we did too quickly and did not compile with the correct OpenACC flag. Note that you also need to adapt the

 write (*,*) "Expected value sum(f3): ", EXPECTED_CUDA_SUM, "actual value:", sum(f3)

below

Copy link
Contributor

Choose a reason for hiding this comment

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

sorry for messing this up

Co-Authored-By: Andreas Jocksch <andreas.jocksch@cscs.ch>
Co-Authored-By: Andreas Jocksch <andreas.jocksch@cscs.ch>
@ajocksch ajocksch merged commit fe56b36 into master Aug 13, 2019
@ajocksch ajocksch deleted the fix_array branch August 13, 2019 08:13
@lucamar lucamar changed the title Fix openacc_cuda_mpi_cppstd.F90 [test] Fix openacc_cuda_mpi_cppstd.F90 Aug 22, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants