lattice / quda

QUDA is a library for performing calculations in lattice QCD on GPUs.
https://lattice.github.io/quda
Other
289 stars 97 forks source link

Removed '#include <complex.h>' #1331

Closed bjoo closed 1 year ago

bjoo commented 1 year ago

To avoid a #define conflict between and gtest.h when using hipcc compiler. This fixes a local issue Venkitesh Ayyar raised on Crysger at OLCF which prevented gauge_path_test.cpp from compiling with the following #define clash:

shared/ayyar/builds_crusher/install_oct17_2022/QUDA/src/quda/tests/googletest/include/gtest/internal/gtest-internal.h:1154:21: error: expected ')'
template <size_t... I, size_t sizeofT>
                    ^
/usr/include/complex.h:53:11: note: expanded from macro 'I'
#define I _Complex_I
          ^
/usr/include/complex.h:48:21: note: expanded from macro '_Complex_I'
#define _Complex_I      (__extension__ 1.0iF)
                         ^
/gpfs/alpine/lgt104/proj-shared/ayyar/builds_crusher/install_oct17_2022/QUDA/src/quda/tests/googletest/include/gtest/internal/gtest-internal.h:1154:21: note: to match this '('
/usr/include/complex.h:53:11: note: expanded from macro 'I'
#define I _Complex_I
          ^
/usr/include/complex.h:48:20: note: expanded from macro '_Complex_I'
#define _Complex_I      (__extension__ 1.0iF)

Also I changed from #include <complex.h> to the more C++ #include <complex>

maddyscientist commented 1 year ago

@bjoo thanks for the fix.

Note it was intentional to include the C complex header not the C++ one, since the QUDA interface is C based. This test might not even need this header at all though?

mathiaswagner commented 1 year ago

Reminds me that we should clean up headers using IWYU at some point.

bjoo commented 1 year ago

Turns out it wasn't needed. I commented it and the code compiled fine. So I just ended up removing it. Makes for a cleaner PR

maddyscientist commented 1 year ago

Just noting that this issue was a gtest issue which has since been fixed. We should update the version of gtest we package with QUDA, or perhaps use CPM if it's available?

kostrzewa commented 1 year ago

@bjoo Thanks for this. Also hit it recently but didn't get around to figuring out where it was coming from.