Tpetra: link errors in Norms with deprecated code OFF

[kddevin]

Bug Report

@trilinos/tpetra

Description

Using the develop branch with Tpetra_ENABLE_DEPRECATED_CODE=OFF, I see the following link errors:

[ 50%] Linking CXX executable TpetraCore_BlockExpNamespace.exe
Undefined symbols for architecture x86_64:
  "void Tpetra::Details::normImpl<int, Kokkos::LayoutLeft, Kokkos::Device<Kokkos::Serial, Kokkos::HostSpace>, int>(int*, Kokkos::View<int const**, Kokkos::LayoutLeft, Kokkos::Device<Kokkos::Serial, Kokkos::HostSpace> > const&, Tpetra::Details::EWhichNorm, Teuchos::ArrayView<unsigned long const> const&, bool, bool, Teuchos::Comm<int> const*)", referenced from:
      void (anonymous namespace)::multiVectorNormImpl<int, int, long long, Kokkos::Compat::KokkosDeviceWrapperNode<Kokkos::Serial, Kokkos::HostSpace> >(Tpetra::MultiVector<int, int, long long, Kokkos::Compat::KokkosDeviceWrapperNode<Kokkos::Serial, Kokkos::HostSpace> >::mag_type*, Tpetra::MultiVector<int, int, long long, Kokkos::Compat::KokkosDeviceWrapperNode<Kokkos::Serial, Kokkos::HostSpace> >&, Tpetra::Details::EWhichNorm) in libtpetra.a(Tpetra_MultiVector_INT_INT_LONG_LONG_SERIAL.cpp.o)
ld: symbol(s) not found for architecture x86_64

Steps to Reproduce

cmake \
-D Trilinos_ENABLE_EXPLICIT_INSTANTIATION:BOOL=ON \
-D CMAKE_BUILD_TYPE:STRING="DEBUG" \
-D CMAKE_VERBOSE_MAKEFILE:BOOL=OFF \
\
-D MPI_BIN_DIR:PATH="/Users/kddevin/InstalledSoftware/openmpi-1.8.3_clang/bin" \
-D TPL_ENABLE_MPI:BOOL=ON \
-D MPI_EXEC_MAX_NUMPROCS:STRING=11 \
\
-D TPL_ENABLE_BinUtils:BOOL=OFF \
-D TPL_ENABLE_Pthread:BOOL=OFF \
\
-D CMAKE_C_FLAGS:STRING="-Wall  -pedantic -Wno-unknown-pragmas -Wno-narrowing -Wno-inline -Wshadow -Wdeprecated-declarations -Wempty-body  -Wignored-qualifiers -Wmissing-field-initializers  -Wsign-compare  -Wtype-limits   -Wuninitialized -Winit-self -fstrict-aliasing -Wno-long-long" \
-D CMAKE_CXX_FLAGS:STRING="-Wall -pedantic -Wno-unknown-pragmas -Wno-narrowing -Wno-delete-non-virtual-dtor -Wno-inline -Wshadow -Wdeprecated-declarations -Wempty-body  -Wignored-qualifiers -Wmissing-field-initializers  -Wsign-compare  -Wtype-limits   -Wuninitialized -Winit-self -fstrict-aliasing" \
\
-D Trilinos_ENABLE_ALL_OPTIONAL_PACKAGES:BOOL=OFF \
-D Trilinos_ENABLE_TESTS:BOOL=OFF \
-D Trilinos_ENABLE_EXAMPLES:BOOL=OFF \
\
-D Trilinos_ENABLE_SHADOW_WARNINGS:BOOL=ON \
-D Trilinos_VERBOSE_CONFIGURE:BOOL=OFF \
-D Trilinos_ENABLE_Fortran:BOOL=OFF \
\
-D Trilinos_ENABLE_Tpetra:BOOL=ON \
-D Tpetra_ENABLE_TESTS:BOOL=ON \
-D Tpetra_ENABLE_EXAMPLES:BOOL=ON \
\
-D Tpetra_ENABLE_DEPRECATED_CODE:BOOL=OFF \
-D Tpetra_INST_INT_LONG_LONG:BOOL=ON \
-D Tpetra_INST_INT_INT:BOOL=OFF \
\
-D Teuchos_ENABLE_STACKTRACE:BOOL=OFF \
-D Teuchos_ENABLE_LONG_LONG_INT:BOOL=ON \
..

[mhoemmen]

FYI, you don't need the option Teuchos_ENABLE_LONG_LONG_INT:BOOL=ON any more; in fact, the option no longer exists.

[mhoemmen]

The cause could be that tpetra/core/src/Tpetra_Details_normImpl.cpp lacks ETI logic for Scalar=int.

[kddevin]

It works OK when deprecated code is ON.

[kddevin]

Are you saying this could be a CMakeLists.txt issue rather than a source code issue?

[mhoemmen]

Hm, I have a build here with Tpetra_ENABLE_DEPRECATED_CODE=OFF and Tpetra_INST_INT_INT=OFF, and nevertheless it looks like TPETRA_INSTANTIATE_SN includes Scalar=int. @kddevin, could you check in your build directory, in packages/tpetra/core/src/TpetraCore_ETIHelperMacros.h, and post the definition of the TPETRA_INSTANTIATE_SN macro? Thanks!

[kddevin]

Thanks for looking at this issue, @mhoemmen .

With Tpetra_ENABLE_DEPRECATED_CODE=ON:

#define TPETRA_INSTANTIATE_SN(INSTMACRO)\
        INSTMACRO( int , Kokkos_Compat_KokkosSerialWrapperNode )\
        INSTMACRO( longlong , Kokkos_Compat_KokkosSerialWrapperNode )\
        INSTMACRO( double , Kokkos_Compat_KokkosSerialWrapperNode )

With Tpetra_ENABLE_DEPRECATED_CODE=OFF:

#define TPETRA_INSTANTIATE_SN(INSTMACRO)\
        INSTMACRO( longlong , Kokkos_Compat_KokkosSerialWrapperNode )\
        INSTMACRO( double , Kokkos_Compat_KokkosSerialWrapperNode )

[mhoemmen]

@kddevin I think I've figured this out.

1. Tpetra uses TpetraCore_ETI_SCALARS for the TPETRA_INSTANTIATE_S* macros.
2. tpetra/CMakeLists.txt only adds int to TpetraCore_ETI_SCALARS if Tpetra_INST_INT_INT=ON.
3. This doesn't break Tpetra::MultiVector or Tpetra::Vector, because they don't use the ETI macros. Instead, they use the .cpp files automatically generated by tpetra/core/src/CMakeLists.txt for instantiation.
4. Tpetra::Details::normImpl uses the ETI macro TPETRA_INSTANTIATE_SN, because it only needs instantiation over Scalar and Node. The ETI system for generating .cpp files (see (3) above) only works for classes that take template parameters (Scalar, LO, GO, Node).

[kddevin]

Thanks, @mhoemmen
Is this a new feature? I have been using the same config script for a few weeks, though I don't know whether I've used it since the recent CMakeLists.txt commits.

[kddevin]

Should we pull @william76 into the conversation?

[mhoemmen]

@kddevin wait a minute or two; I am writing a temporary solution for you.

[mhoemmen]

The easiest work-around would be to add int instantiations to Tpetra_Details_normImpl.cpp, as follows:

#if defined(HAVE_TPETRA_INST_INT_INT)
// don't need to do anything; Scalar=int is already added
# define TPETRA_DETAILS_NORMIMPL_INSTANT_INT( NODE )
#else 
# define TPETRA_DETAILS_NORMIMPL_INSTANT_INT( NODE ) \
  TPETRA_DETAILS_NORMIMPL_INSTANT( int, NODE )
#endif 

namespace Tpetra {

  TPETRA_ETI_MANGLING_TYPEDEFS()

  TPETRA_INSTANTIATE_SN( TPETRA_DETAILS_NORMIMPL_INSTANT )
  TPETRA_INSTANTIATE_N( TPETRA_DETAILS_NORMIMPL_INSTANT_INT )

#ifdef HAVE_TPETRA_INST_CUDA

  using cuda_host_mirror_device_type =
    Kokkos::Device<Kokkos::DefaultHostExecutionSpace,
                   Kokkos::CudaUVMSpace>;

#define TPETRA_DETAILS_NORMIMPL_INSTANT_CUDAHOSTMIRROR( S ) \
  TPETRA_DETAILS_NORMIMPL_INSTANT( S, cuda_host_mirror_device_type )

  TPETRA_INSTANTIATE_S( TPETRA_DETAILS_NORMIMPL_INSTANT_CUDAHOSTMIRROR )
#if ! defined(HAVE_TPETRA_INST_INT_INT)
  TPETRA_DETAILS_NORMIMPL_INSTANT_CUDAHOSTMIRROR( int )
#endif 

#endif // HAVE_TPETRA_INST_CUDA

} // namespace Tpetra

[kddevin]

Wow, thanks, @mhoemmen
I will add this and test both with/without deprecated code.
I can PR the changes then, if you think that is the right thing to do.

[mhoemmen]

@kddevin wrote:

I can PR the changes then, if you think that is the right thing to do.

Please do; I have another thing on the fire at the moment. Thanks!

[kddevin]

@mhoemmen
Before we attempt a fix to #4962 , please help me understand the problem.

As I understand it:

When Trilinos instantiated two global ordinal types by default, it added global ordinal types int and long long to the instantiated scalar types.

With Tpetra_ENABLE_DEPRECATED_CODE=OFF, the default global ordinal type is then long long. Tpetra adds long long to the instantiated scalar types.

But then something in Tpetra is still trying to run with MultiVector<int, int, long long>.
Is the problem the instantiations? Or that something in Tpetra is trying to do MultiVector<int, int, long long> when it should do MultiVector<GO, int, long long>?

That is, should we back out the instantiation changes and, instead, fix the call to normImpl?

[kddevin]

OK, I see. MultiVector is instantiated with scalar=int but this normImpl function is not.
The question: if we are allowing only one global ordinal type, should we instantiate MultiVector (et al.) with only one global ordinal type?
I cannot find where TPETRA_MULTIVECTOR_INSTANT is used; I can only see where it is defined. Where is it used? Where does the MultiVector<int, int, long long> get instantiated?

For now, I am going to revert the changes above so that @ikalash 's tests will pass.
Note to @trilinos/tpetra developers: with the default Tpetra_INST_INT_LONG_LONG, I will get these link errors again. Do others see these? The short-term workaround is to use Tpetra_INST_INT_INT instead.

I will revisit this later this weekend.

[ikalash]

Thanks @kddevin !

[mhoemmen]

@kddevin wrote:

But then something in Tpetra is still trying to run with MultiVector<int, int, long long>.

It's actually downstream of Tpetra, in MueLu. MueLu uses Vector (and therefore MultiVector) with Scalar=int to communicate MPI process ranks (which have type int). This is an old trick; ML also does this (but with ML's native boundary exchange, not with Epetra).

This is why Tpetra treats MultiVector ETI as a special case, and always includes Scalar=int, regardless of whether Tpetra_INST_INT_INT is ON.

MueLu could just cast the process ranks to GO and use MultiVector<GO, ...>. However, economizing instantiations was never an historical priority for Trilinos developers ;-)

[kddevin]

OK; that explains why MultiVector has scalar = int. But how is it done? I don't see where TPETRA_MULTIVECTOR_INSTANT is used anywhere. How is the MultiVector instantiation enabled?

Also, what code change caused the bug? Is normImpl new?
Did we miss this error because we need tests with Tpetra_INST_INT_INT=OFF?

[mhoemmen]

@kddevin wrote:

OK; that explains why MultiVector has scalar = int. But how is it done?

Tpetra has two ETI systems. The first and original one predates me. Read Trilinos/packages/tpetra/core/cmake/README for an explanation (that I wrote, after reverse-engineering what it did). In short, it uses macros that CMake generates. Look in the build (not source) directory, at packages/tpetra/core/src/TpetraCore_ETIHelperMacros.h, to see the macros. The macros take a macro as an argument, and iterate over all enabled template argument combinations, invoking the input macro for each.

I wrote the second ETI system when build times with ETI enabled became unacceptably large. (Laptops were running out of memory building a single .cpp file.) The issue is that the first system's ETI macros (e.g., TPETRA_INSTANTIATE_SLGN) iterate over all enabled template argument combinations, in one place. That puts all the instantiations in one file. Instead, what I did was use CMake (in Trilinos/packages/tpetra/core/src/CMakeLists.txt) to generate one .cpp file in the build directory for each instantiation. The .cpp files still use the Tpetra class macro defined in the _def.hpp header file, and the mangled typedefs in TpetraCore_ETIHelpersMacros.h.

The two systems can coexist just fine. Classes and functions must opt into ETI explicitly, and they can use either system (but not both, of course!). The second system currently only works for classes that take all four (S, LO, GO, N) template parameters, which is why I had to use the first system for Tpetra::Details::normImpl.

Christian Trott uses a different system for kokkos-kernels. It's a variant of the second system, but Christian uses a Python script to generate the .cpp files statically, and checks all of them into the source repository. This is why you see a ton of .cpp files when building kokkos-kernels. Most of them build nothing (they use macros to enable only the instantiation(s) in the file), but the build must still invoke the compiler on each file. That's one disadvantage of this system. Another disadvantage is that if you later add or remove types, you have to rerun the Python script by hand and update the list of files. However, the advantage is that more people understand Python than CMake ;-) .

I still haven't quite answered your question. In Tpetra's version of the second system, MultiVector is a special case. You can see the logic for adding Scalar=int here:

Trilinos/packages/tpetra/core/src/CMakeLists.txt

Line 295 in 8c5bac7

LIST(APPEND MultiVector_ETI_SCALARS "int")

[kddevin]

@mhoemmen I'm still having troubles with normImpl instantiation. I get link errors with a stokhos build.

Using the ATDM testing framework, I do

source ../cmake/std/atdm/load-env.sh  gnu-7.2.0-openmp-release-debug
MPI_EXEC=`which mpiexec`
cmake \
  -GNinja \
  -DMPI_EXEC=${MPI_EXEC} \
  -DTrilinos_CONFIGURE_OPTIONS_FILE:STRING=cmake/std/atdm/ATDMDevEnv.cmake \
  -DTrilinos_ENABLE_TESTS=ON -DTrilinos_ENABLE_Teuchos=ON \
  -DTrilinos_ENABLE_Anasazi=ON -DTrilinos_ENABLE_Belos=ON \
  -DTrilinos_ENABLE_ROL=ON -DTrilinos_ENABLE_Stokhos=ON \
  -DTrilinos_ENABLE_Tempus=ON \
.. |& tee OUTPUT.CMAKE

Here are the link errors:

packages/stokhos/src/libstokhos_tpetra_mp_16_openmp.a(Tpetra_MultiVector_MP_Vector_16_OpenMP.cpp.o): In function `void (anonymous namespace)::multiVectorNormImpl<Sacado::MP::Vector<Stokhos::StaticFixedStorage<int, double, 16, Kokkos::OpenMP> >, int, int, Kokkos::Compat::KokkosDeviceWrapperNode<Kokkos::OpenMP, Kokkos::HostSpace> >(Tpetra::MultiVector<Sacado::MP::Vector<Stokhos::StaticFixedStorage<int, double, 16, Kokkos::OpenMP> >, int, int, Kokkos::Compat::KokkosDeviceWrapperNode<Kokkos::OpenMP, Kokkos::HostSpace> >::mag_type*, Tpetra::MultiVector<Sacado::MP::Vector<Stokhos::StaticFixedStorage<int, double, 16, Kokkos::OpenMP> >, int, int, Kokkos::Compat::KokkosDeviceWrapperNode<Kokkos::OpenMP, Kokkos::HostSpace> >&, Tpetra::Details::EWhichNorm)':
/home/kddevin/tmp/tmp/Trilinos/Obj_atdm/../packages/tpetra/core/src/Tpetra_MultiVector_def.hpp:388: undefined reference to `_ZN6Tpetra7Details8normImplIN6Sacado2MP6VectorIN7Stokhos18StaticFixedStorageIidLi16EN6Kokkos6OpenMPEEEEENS7_10LayoutLeftENS7_6DeviceIS8_NS7_9HostSpaceEEESA_EEvPT2_RKNS7_4ViewIPPKT_JT0_T1_EEENS0_10EWhichNormERKN7Teuchos9ArrayViewIKmEEbbPKNSS_4CommIiEE'
/home/kddevin/tmp/tmp/Trilinos/Obj_atdm/../packages/tpetra/core/src/Tpetra_MultiVector_def.hpp:375: undefined reference to `_ZN6Tpetra7Details8normImplIN6Sacado2MP6VectorIN7Stokhos18StaticFixedStorageIidLi16EN6Kokkos6OpenMPEEEEENS7_10LayoutLeftENS7_6DeviceIS8_NS7_9HostSpaceEEESA_EEvPT2_RKNS7_4ViewIPPKT_JT0_T1_EEENS0_10EWhichNormERKN7Teuchos9ArrayViewIKmEEbbPKNSS_4CommIiEE'

packages/stokhos/src/libstokhos_tpetra_pce_openmp.a(Tpetra_MultiVector_UQ_PCE_OpenMP.cpp.o): In function `void (anonymous namespace)::multiVectorNormImpl<Sacado::UQ::PCE<Stokhos::DynamicStorage<int, double, Kokkos::OpenMP> >, int, int, Kokkos::Compat::KokkosDeviceWrapperNode<Kokkos::OpenMP, Kokkos::HostSpace> >(Tpetra::MultiVector<Sacado::UQ::PCE<Stokhos::DynamicStorage<int, double, Kokkos::OpenMP> >, int, int, Kokkos::Compat::KokkosDeviceWrapperNode<Kokkos::OpenMP, Kokkos::HostSpace> >::mag_type*, Tpetra::MultiVector<Sacado::UQ::PCE<Stokhos::DynamicStorage<int, double, Kokkos::OpenMP> >, int, int, Kokkos::Compat::KokkosDeviceWrapperNode<Kokkos::OpenMP, Kokkos::HostSpace> >&, Tpetra::Details::EWhichNorm)':
/home/kddevin/tmp/tmp/Trilinos/Obj_atdm/../packages/tpetra/core/src/Tpetra_MultiVector_def.hpp:388: undefined reference to `_ZN6Tpetra7Details8normImplIN6Sacado2UQ3PCEIN7Stokhos14DynamicStorageIidN6Kokkos6OpenMPEEEEENS7_10LayoutLeftENS7_6DeviceIS8_NS7_9HostSpaceEEEdEEvPT2_RKNS7_4ViewIPPKT_JT0_T1_EEENS0_10EWhichNormERKN7Teuchos9ArrayViewIKmEEbbPKNSS_4CommIiEE'
/home/kddevin/tmp/tmp/Trilinos/Obj_atdm/../packages/tpetra/core/src/Tpetra_MultiVector_def.hpp:375: undefined reference to `_ZN6Tpetra7Details8normImplIN6Sacado2UQ3PCEIN7Stokhos14DynamicStorageIidN6Kokkos6OpenMPEEEEENS7_10LayoutLeftENS7_6DeviceIS8_NS7_9HostSpaceEEEdEEvPT2_RKNS7_4ViewIPPKT_JT0_T1_EEENS0_10EWhichNormERKN7Teuchos9ArrayViewIKmEEbbPKNSS_4CommIiEE'

[mhoemmen]

@kddevin We should just forget about ETI for normImpl. It's not clear that it's worth the trouble.

[kddevin]

@mhoemmen does one do that by just yanking out all macros mentioning NORMIMPL and purgin Tpetra_Details_normImpl.cpp?

[mhoemmen]

@kddevin We would need to do the following:

1. Create a new file Tpetra_Details_normImpl.hpp in the source directory (Trilinos/packages/tpetra/core/src).
2. Copy the declarations and definitions of Tpetra::Details::normImpl into this file.
3. Delete Tpetra_Details_normImpl_def.hpp, Tpetra_Details_normImpl_decl.hpp, and Tpetra_Details_normImpl.cpp.
4. Get rid of the explicit instantiations (use of NORMIMPL macros).

We need to do 1-3 because otherwise, if ETI is enabled, #include "Tpetra_Details_normImpl.hpp" will only include Tpetra_Details_normImpl_decl.hpp.

[mhoemmen]

@rrdrake @ajpowelsnl FYI