search

trilinos / Trilinos

Primary repository for the Trilinos Project
https://trilinos.org/
Other
1.22k stars 568 forks source link

Teuchos: BLAS specify the return type of Complex blas calls #11010

Closed jjellio closed 1 year ago

jjellio commented 2 years ago

Question

@trilinos/teuchos

I've got alot of spam (warnings) about C-linkage and return values of complex blas functions. E.g.,

/g/g20/jjellio/src/github/Trilinos-a/packages/teuchos/numerics/src/Teuchos_BLAS_wrappers.hpp:198:29: warning: 'zdotc_' has C-linkage specified, but returns user-defined type 'std::complex<double>' which is incompatible with C [-Wreturn-type-c-linkage]
std::complex<double> PREFIX ZDOT_F77(const int* n, const std::complex<double> x[], const int* incx, const std::complex<double> y[], const int* incy);
                            ^
/g/g20/jjellio/src/github/Trilinos-a/packages/teuchos/numerics/src/Teuchos_BLAS_wrappers.hpp:101:21: note: expanded from macro 'ZDOT_F77'
#define ZDOT_F77    F77_BLAS_MANGLE(zdotc,ZDOTC)
                    ^
/tmp/jjellio/build/cce-14.0.2_prgenv-cray_rocm-5.2.3_mpich-8.1.18_pure-cce_hip_amd-inlall-NOfunc-gfx90a_serial_atdm_complex_opt-g_libonly-FIXES/trilinos/packages/ml/src/ml_config.h:8:37: note: expanded from macro 'F77_BLAS_MANGLE'
 #define F77_BLAS_MANGLE(name,NAME) name ## _
                                    ^
<scratch space>:277:1: note: expanded from here
zdotc_
^

Is there a feature in Teuchos' wrappers that handles the return type better? I skimmed the code/cmake and didn't see anything. Intel discusses this, e.g., Calling complex blas through C++. And they aren't using the return value of the blas calls. I think Trilinos prefers to call the Fortran symbols directly, so it requires C-linkages for those.

@bartlettroscoe

bartlettroscoe commented 2 years ago

@jjellio, the last commit I can find related to complex return times is dc26ca9db2695d5998916cc2cab3367de557fa03 6 years ago. Looks the solution is to use float _Complex and double _Complex?

Now that C++11 is required (for many years now), perhaps all of the BLAS and LAPACK wrappers that return complex types should just be hard-coded to use float _Complex and double _Complex?

jjellio commented 2 years ago

Oh interesting. So, this is a side effect of moving to CXX=14 or 17? I didn't realzie HAVE_TEUCHOSCORE_CXX11 was not defined. Maybe, that macro should be HAVE_TEUCHOSCORE_ATLEAST_CXX11 or something? Or just define HAVE_TEUCHOSCORE_CXX11 if CXX>= 11 is used?

jjellio commented 2 years ago

So weird, https://github.com/trilinos/Trilinos/blob/64d50199f342860b00d16cb78cb5730de66438b9/packages/teuchos/core/src/CMakeLists.txt#L29

That should be defining HAVE_TEUCHOSCORE_CXX11 - so why would my build be evaluating the off Branch... I have a CMakeCache posted around here somewhere..

jjellio commented 2 years ago

HAVE_TEUCHOSCORE_CXX11 is not defined in my CMakeCache.txt (Attached) tuechos-cxx11-CmakeCache.txt

Configure Line ``` cmake \ "-GNinja" \ "-DCMAKE_BUILD_TYPE:STRING=Release" \ "-DTrilinos_ENABLE_TrilinosBuildStats=OFF" \ "-DTrilinos_ENABLE_BUILD_STATS=OFF" \ "-DTrilinosBuildStats_ENABLE_TESTS=OFF" \ "-DBUILD_SHARED_LIBS:BOOL=OFF" \ "-DTrilinos_ENABLE_Teuchos=ON" \ "-DTrilinos_ENABLE_Percept=OFF" \ "-DTrilinos_ENABLE_ALL_PACKAGES=ON" \ "-DKokkos_ARCH_ZEN3:BOOL=ON" \ "-DKokkos_ENABLE_SERIAL:BOOL=ON" \ "-DTpetra_INST_SERIAL:BOOL=ON" \ "-DKokkos_ENABLE_OPENMP:BOOL=OFF" \ "-DTpetra_INST_OPENMP:BOOL=OFF" \ "-DTrilinos_ENABLE_OpenMP:BOOL=OFF" \ "-DKokkos_ENABLE_HIP:BOOL=ON" \ "-DTpetra_INST_HIP:BOOL=ON" \ "-DKokkos_ARCH_VEGA90A:BOOL=ON" \ "-DCMAKE_EXE_LINKER_FLAGS=-x none " \ "-DCMAKE_SHARED_LINKER_FLAGS=-x none" \ "-DCMAKE_LINKER=/opt/cray/pe/craype/2.7.17/bin/CC" \ "-DCMAKE_CXX_LINK_EXECUTABLE= -x none -hsystem_alloc -o " \ "-DCMAKE_CXX_CREATE_SHARED_LIBRARY= -x none -hsystem_alloc -o " \ "-DCMAKE_Fortran_LINK_EXECUTABLE= -x none -hsystem_alloc -o " \ "-DCMAKE_Fortran_CREATE_SHARED_LIBRARY= -hsystem_alloc -o " \ "-DCMAKE_CXX_COMPILER=/opt/rocm-5.2.3/bin/amdclang++" \ "-DCMAKE_C_COMPILER=/opt/cray/pe/craype/2.7.17/bin/cc" \ "-DCMAKE_Fortran_COMPILER=/opt/cray/pe/craype/2.7.17/bin/ftn" \ "-DCMAKE_CXX_FLAGS=--offload-arch=gfx90a --rocm-path=/opt/rocm-5.2.3 -x hip -mllvm -amdgpu-early-inline-all=true -mllvm -amdgpu-function-calls=false -I/opt/cray/pe/mpich/8.1.18/ofi/crayclang/10.0/include -I/opt/rocm-5.2.3/include -g " \ "-DCMAKE_Fortran_FLAGS=-I/opt/cray/pe/mpich/8.1.18/ofi/crayclang/10.0/include -I/opt/rocm-5.2.3/include -hsystem_alloc " \ "-DCMAKE_C_FLAGS=-I/opt/cray/pe/mpich/8.1.18/ofi/crayclang/10.0/include -I/opt/rocm-5.2.3/include " \ "-DTrilinos_EXTRA_LINK_FLAGS=-L/opt/rocm-5.2.3/lib -Wl,-rpath,/opt/rocm-5.2.3/lib -lamdhip64 -L/opt/cray/pe/mpich/8.1.18/ofi/crayclang/10.0/lib -Wl,-rpath,/opt/cray/pe/mpich/8.1.18/ofi/crayclang/10.0/lib -lmpi -L/opt/cray/pe/mpich/8.1.18/gtl/lib -Wl,-rpath,/opt/cray/pe/mpich/8.1.18/gtl/lib -lmpi_gtl_hsa " \ "-DTrilinos_ENABLE_Fortran:BOOL=ON" \ "-DTrilinos_ENABLE_TrilinosATDMConfigTests:BOOL=OFF" \ "-DTrilinos_ENABLE_TrilinosFrameworkTests=OFF" \ "-DTrilinos_ENABLE_MiniTensor=OFF" \ "-DTrilinos_ENABLE_Isorropia=OFF" \ "-DTrilinos_ENABLE_KokkosExample=OFF" \ "-DTrilinos_ENABLE_Domi=OFF" \ "-DTrilinos_ENABLE_Pliris=OFF" \ "-DTrilinos_ENABLE_Komplex=OFF" \ "-DTrilinos_ENABLE_FEI=OFF" \ "-DTrilinos_ENABLE_TriKota=OFF" \ "-DTrilinos_ENABLE_Compadre=OFF" \ "-DTrilinos_ENABLE_STKClassic=OFF" \ "-DTrilinos_ENABLE_STKSearchUtil=OFF" \ "-DTrilinos_ENABLE_STKUnit_tests=OFF" \ "-DTrilinos_ENABLE_STKDoc_tests=OFF" \ "-DTrilinos_ENABLE_STKExp=OFF" \ "-DTrilinos_ENABLE_Moertel=OFF" \ "-DTrilinos_ENABLE_Stokhos=OFF" \ "-DTrilinos_ENABLE_MOOCHO=OFF" \ "-DTrilinos_ENABLE_PyTrilinos=OFF" \ "-DTrilinos_ENABLE_TrilinosCouplings=OFF" \ "-DTrilinos_ENABLE_Pike=OFF" \ "-DShyLU_DD_ENABLE_BDDC=OFF" \ "-DTrilinos_ENABLE_PyTrilinos=OFF" \ "-DKOKKOSKERNELS_TPL_BLAS_RETURN_COMPLEX=OFF" \ "-DTacho_ENABLE_INT_INT:BOOL=ON" \ "-DTrilinos_ENABLE_COMPLEX_DOUBLE:BOOL=ON" \ "-DTPL_ENABLE_BinUtils:BOOL=OFF" \ "-DBinUtils_INCLUDE_DIRS=/p/lustre1/jjellio/spack/install/cray-rhel8-zen3/cce-14.0.0/binutils-2.38-qtwpwyu4a23d5534lgbjyw6zwmorh44b/include" \ "-DBinUtils_LIBRARY_DIRS=/p/lustre1/jjellio/spack/install/cray-rhel8-zen3/cce-14.0.0/binutils-2.38-qtwpwyu4a23d5534lgbjyw6zwmorh44b/lib" \ "-DTPL_ENABLE_BLAS:BOOL=ON" \ "-DTPL_ENABLE_LAPACK:BOOL=ON" \ "-DBLAS_LIBRARY_NAMES=sci_cray" \ "-DBLAS_INCLUDE_DIRS=/opt/cray/pe/libsci/22.08.1.1/CRAY/9.0/x86_64/include" \ "-DBLAS_LIBRARY_DIRS=/opt/cray/pe/libsci/22.08.1.1/CRAY/9.0/x86_64/lib" \ "-DLAPACK_LIBRARY_NAMES=sci_cray" \ "-DLAPACK_INCLUDE_DIRS=/opt/cray/pe/libsci/22.08.1.1/CRAY/9.0/x86_64/include" \ "-DLAPACK_LIBRARY_DIRS=/opt/cray/pe/libsci/22.08.1.1/CRAY/9.0/x86_64/lib" \ "-DTPL_ENABLE_Boost:BOOL=ON" \ "-DTPL_ENABLE_BoostLib:BOOL=ON" \ "-DBoost_INCLUDE_DIRS=/p/lustre1/jjellio/spack/install/cray-rhel8-zen3/cce-14.0.0/boost-1.79.0-tnsjrvavksgzemju6s4gbptglnwoo4ye/include" \ "-DBoost_LIBRARY_DIRS=/p/lustre1/jjellio/spack/install/cray-rhel8-zen3/cce-14.0.0/boost-1.79.0-tnsjrvavksgzemju6s4gbptglnwoo4ye/lib" \ "-DBoostLib_INCLUDE_DIRS=/p/lustre1/jjellio/spack/install/cray-rhel8-zen3/cce-14.0.0/boost-1.79.0-tnsjrvavksgzemju6s4gbptglnwoo4ye/include" \ "-DBoostLib_LIBRARY_DIRS=/p/lustre1/jjellio/spack/install/cray-rhel8-zen3/cce-14.0.0/boost-1.79.0-tnsjrvavksgzemju6s4gbptglnwoo4ye/lib" \ "-DTPL_ENABLE_METIS:BOOL=ON" \ "-DTPL_ENABLE_ParMETIS:BOOL=ON" \ "-DTPL_METIS_INCLUDE_DIRS=/p/lustre1/jjellio/spack/install/cray-rhel8-zen3/cce-14.0.0/metis-5.1.0-fpif6qy3otttnm26lzbqvkqwjnzgrw6b/include" \ "-DTPL_METIS_LIBRARY_DIRS=/p/lustre1/jjellio/spack/install/cray-rhel8-zen3/cce-14.0.0/metis-5.1.0-fpif6qy3otttnm26lzbqvkqwjnzgrw6b/lib" \ "-DTPL_ParMETIS_INCLUDE_DIRS=/p/lustre1/jjellio/spack/install/cray-rhel8-zen3/cce-14.0.0/parmetis-4.0.3-jpgzminxnku2kjev6oh3u5wj5yh4wdmz/include;/p/lustre1/jjellio/spack/install/cray-rhel8-zen3/cce-14.0.0/metis-5.1.0-fpif6qy3otttnm26lzbqvkqwjnzgrw6b/include" \ "-DTPL_ParMETIS_LIBRARY_DIRS=/p/lustre1/jjellio/spack/install/cray-rhel8-zen3/cce-14.0.0/parmetis-4.0.3-jpgzminxnku2kjev6oh3u5wj5yh4wdmz/lib;/p/lustre1/jjellio/spack/install/cray-rhel8-zen3/cce-14.0.0/metis-5.1.0-fpif6qy3otttnm26lzbqvkqwjnzgrw6b/lib" \ "-DTPL_ENABLE_CGNS:BOOL=ON" \ "-DCGNS_INCLUDE_DIRS=/p/lustre1/jjellio/spack/install/cray-rhel8-zen3/cce-14.0.0/cgns-4.3.0-y5srwcogbr45zyvosxb2o4t3h7ejr4mp/include" \ "-DCGNS_LIBRARY_DIRS=/p/lustre1/jjellio/spack/install/cray-rhel8-zen3/cce-14.0.0/cgns-4.3.0-y5srwcogbr45zyvosxb2o4t3h7ejr4mp/lib" \ "-DTPL_ENABLE_HDF5:BOOL=ON" \ "-DHDF5_LIBRARY_DIRS=/p/lustre1/jjellio/spack/install/cray-rhel8-zen3/cce-14.0.0/hdf5-1.10.7-4muxy5a2pluisnyl6viqey45adgmoyso/lib;/p/lustre1/jjellio/spack/install/cray-rhel8-zen3/cce-14.0.0/zlib-1.2.12-7rdn5lyw7s67v4ofde5xwfakbrnkuvvo/lib" \ "-DHDF5_INCLUDE_DIRS=/p/lustre1/jjellio/spack/install/cray-rhel8-zen3/cce-14.0.0/hdf5-1.10.7-4muxy5a2pluisnyl6viqey45adgmoyso/include" \ "-DHDF5_LIBRARY_NAMES=hdf5_hl;hdf5;z;dl" \ "-DTPL_ENABLE_Netcdf:BOOL=ON" \ "-DNetcdf_LIBRARY_DIRS=/p/lustre1/jjellio/spack/install/cray-rhel8-zen3/cce-14.0.0/zlib-1.2.12-7rdn5lyw7s67v4ofde5xwfakbrnkuvvo/lib;/p/lustre1/jjellio/spack/install/cray-rhel8-zen3/cce-14.0.0/boost-1.79.0-tnsjrvavksgzemju6s4gbptglnwoo4ye/lib;/p/lustre1/jjellio/spack/install/cray-rhel8-zen3/cce-14.0.0/netcdf-c-4.8.1-3tiq2lvtmgjpu5yzlhz4dslij3whlqct/lib;/p/lustre1/jjellio/spack/install/cray-rhel8-zen3/cce-14.0.0/parallel-netcdf-1.12.2-nuvpwpf4tiljvo366c3jmmcsa27fvarm/lib;/p/lustre1/jjellio/spack/install/cray-rhel8-zen3/cce-14.0.0/hdf5-1.10.7-4muxy5a2pluisnyl6viqey45adgmoyso/lib;/p/lustre1/jjellio/spack/install/cray-rhel8-zen3/cce-14.0.0/libzip-1.2.0-bhasbkz5ba6gpqfhgijr3yxefpdkh6nz/lib" \ "-DNetcdf_LIBRARY_NAMES=netcdf;pnetcdf;z;zip;hdf5_hl;hdf5" \ "-DNetcdf_INCLUDE_DIRS=/p/lustre1/jjellio/spack/install/cray-rhel8-zen3/cce-14.0.0/netcdf-c-4.8.1-3tiq2lvtmgjpu5yzlhz4dslij3whlqct/include;/p/lustre1/jjellio/spack/install/cray-rhel8-zen3/cce-14.0.0/parallel-netcdf-1.12.2-nuvpwpf4tiljvo366c3jmmcsa27fvarm/include" \ "-DTPL_ENABLE_SuperLUDist:BOOL=OFF" \ "-DSuperLUDist_INCLUDE_DIRS=/p/lustre1/jjellio/spack/install/cray-rhel8-zen3/cce-14.0.0/superlu-dist-6.4.0-ykwgds7uru2x3ehhxqxbfea5q5m6ieiy/include" \ "-DSuperLUDist_LIBRARY_DIRS=/p/lustre1/jjellio/spack/install/cray-rhel8-zen3/cce-14.0.0/superlu-dist-6.4.0-ykwgds7uru2x3ehhxqxbfea5q5m6ieiy/lib" \ "-DTPL_ENABLE_MPI:BOOL=ON" \ "-DMPI_USE_COMPILER_WRAPPERS=OFF" \ "-DMPI_EXEC_NUMPROCS_FLAG=--srun_clean;--srun_pause=1;--mpibind=off;--exclusive;-c8;--gpus-per-task=1;-n" \ "-DMPI_EXEC=/p/lustre1/jjellio/spack/srun_wrap" \ "-DTPL_ENABLE_DLlib:BOOL=ON" \ "-DDLlib_INCLUDE_DIRS=/opt/rocm-5.2.3/include" \ "-DDLlib_LIBRARY_DIRS=/opt/rocm-5.2.3/lib" \ "-DDLlib_LIBRARY_NAMES=dl;hipsolver;rocsolver;hipblas;rocblas;hipsparse;rocsparse;amd_comgr;hsa-runtime64;amdhip64" \ "-DTPL_ENABLE_Matio=OFF" \ "-DTPL_ENABLE_X11=OFF" \ "-DTrilinos_MAKE_INSTALL_GROUP_READABLE:BOOL=TRUE" \ "-DTrilinos_MAKE_INSTALL_GROUP_WRITABLE:BOOL=FALSE" \ "-DTrilinos_MAKE_INSTALL_WORLD_READABLE:BOOL=TRUE" \ "-DCMAKE_INSTALL_PREFIX=/p/lustre1/jjellio/installs/rzvernal/cce-14.0.2_prgenv-cray_rocm-5.2.3_mpich-8.1.18_hip_amd-inlall-NOfunc-gfx90a_serial_atdm_complex_opt-g_libonly_static" \ /g/g20/jjellio/src/github/Trilinos ```
bartlettroscoe commented 2 years ago

Oh interesting. So, this is a side effect of moving to CXX=14 or 17? I didn't realzie HAVE_TEUCHOSCORE_CXX11 was not defined. Maybe, that macro should be HAVE_TEUCHOSCORE_ATLEAST_CXX11 or something? Or just define HAVE_TEUCHOSCORE_CXX11 if CXX>= 11 is used?

Or just take the ifdefs out all together and hard-code the branch that works with C++11+. There is actually a tool that will do that: unifdef.

jjellio commented 2 years ago

But why isn't it being defined? The logic should be defining it.

And I agree - subtractive changes are as-good or better than additive. (e.g., let's delete things) - But it looks like something fundamental is wrong if that define is not present.

jjellio commented 2 years ago

Doh, I was confused. I see where the problem is https://github.com/trilinos/Trilinos/blob/dc26ca9db2695d5998916cc2cab3367de557fa03/packages/teuchos/numerics/src/Teuchos_BLAS_wrappers.hpp#L198

It seems this could just be set to double _Complex or float _Complex. I can put a PR for that

jjellio commented 1 year ago

@bartlettroscoe this still hasn't gone through, and I don't think "RETEST" will fix it, as the problem appears to be perpetually broken tests. Is my read on this correct? Should we add retest?

bartlettroscoe commented 1 year ago

@jjellio, you mean for PR #11013? I don't think you will see those test failures anymore (except for the random test on the 'vortex' build that will always be possible and there is nothing we can do about, see #11109).