Project

General

Profile

Bug #1157

float/real mismatches in cuda code

Added by Peter Kasson over 6 years ago. Updated about 6 years ago.

Status:
Closed
Priority:
Normal
Assignee:
Category:
mdrun
Target version:
Affected version - extra info:
release-4-6
Affected version:
Difficulty:
uncategorized
Close

Description

I came across this when I accidentally tried to compile double. But this looks iffy to me. Am I missing something?

nbnxn_cuda.cu:
void nbnxn_cuda_wait_gpu(nbnxn_cuda_ptr_t cu_nb,
const nbnxn_atomdata_t *nbatom,
int flags, int aloc,
float *e_lj, float *e_el, rvec *fshift)

nbnxn_cuda.cu:
void nbnxn_cuda_wait_gpu(nbnxn_cuda_ptr_t cu_nb,
const nbnxn_atomdata_t * nbatom,
int flags, int aloc,
real *e_lj, real *e_el,
rvec *fshift) FUNC_TERM

Associated revisions

Revision 37985183 (diff)
Added by Peter Kasson over 6 years ago

Declaration-definition consistency nbxn_cuda_wait_gpu.

Changed float->real in nbnxn_cuda_wait_gpu() for consistency between
function declaration and function definition. Added note that this is
only implemented for single-precision at the moment. CMake will
complain if GMX_GPU and GMX_DOUBLE are both set.
Addresses Bug #1157.

Change-Id: Ide495cbaba6d120d91f106c6a87ca04e46a2f5a8

Revision 68bd2ee6 (diff)
Added by Peter Kasson over 6 years ago

Declaration-definition consistency nbxn_cuda_wait_gpu.

Changed float->real in nbnxn_cuda_wait_gpu() for consistency between
function declaration and function definition. Added note that this is
only implemented for single-precision at the moment. CMake will
complain if GMX_GPU and GMX_DOUBLE are both set.
Addresses Bug #1157.

Change-Id: Ide495cbaba6d120d91f106c6a87ca04e46a2f5a8

History

#1 Updated by Szilárd Páll over 6 years ago

This is indeed not very nice, but in single-precision builds real is typedef-ed to float anyway, so I don't think it is a major concern - as long as we don't have doulbe or some mixed precision modes supported on GPUs.

#2 Updated by Peter Kasson over 6 years ago

Part it comes down to how we want the double-precision compile failure mode to look. As is, this causes compile errors.
If I read correctly, it looks like we detect GMX_GPU and GMX_DOUBLE in cmake/gmxManageGPU.cmake and throw an error.
This sounds like the right behavior.

So it isn't a real issue, but for code cleanliness I might suggest consistent usage (either real or float consistently) in the code.
I'm happy to implement it and send you a CL for review. Do you have a preference for real vs. float? (real would help future-proof the code in case we want to support double in the future and also be more consistent with the rest of our codebase. float would make the float-only nature of the current CUDA code more explicit.)

#3 Updated by Szilárd Páll over 6 years ago

  • Category set to mdrun

Indeed, for code cleanliness it's nest if this issues is solved. We have been discussing that it might be minor amount of effort to enable double-precision support in the CUDA kernels, but it has to be seen ho much will the performance at least on GPUs which are not crippled (i.e most GeForce and K10) suffer from the need to emulate double atomic ops (which is not supported natively).

#4 Updated by Mark Abraham over 6 years ago

  • Target version deleted (4.6.1)
  • Affected version set to 4.6.1

#5 Updated by Teemu Murtola over 6 years ago

  • Status changed from New to Resolved
  • Assignee set to Peter Kasson
  • Target version set to 4.6.x

Had been resolved by the linked commit, probably for 4.6.2.

#6 Updated by Mark Abraham about 6 years ago

  • Status changed from Resolved to Closed
  • Target version changed from 4.6.x to 4.6.4

Also available in: Atom PDF