Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Tpetra: Bad interaction with Kokkos ETI with XL compiler #8418

Closed
tcfisher opened this issue Dec 3, 2020 · 7 comments
Closed

Tpetra: Bad interaction with Kokkos ETI with XL compiler #8418

tcfisher opened this issue Dec 3, 2020 · 7 comments
Labels
client: SPARC Issues related to or needed more specifically by the ATDM SPARC code pkg: Tpetra pkg: Xpetra type: bug The primary issue is a bug in Trilinos code or tests

Comments

@tcfisher
Copy link
Contributor

tcfisher commented Dec 3, 2020 •
edited by jhux2
Loading

Bug Report

@trilinos/tpetra
@jhux2
@brian-kelley

Description

SPARC is failing to link on vortex pwr9 seemingly because of the change in TpetraExt_MatrixMatrix_def.hpp from 75333e4. The XL compiler is the only place it has appeared. The error is as follows:

/projects/atdm_devops/trilinos_installs/2020-12-02/ats2-xl-2020.03.18_spmpi-rolling_serial_static_opt/lib/libtpetraext.a(TpetraExt_MatrixMatrix.cpp.o):(.toc+0x68): undefined reference to `KokkosSparse::Experimental::SortedCountEntries<unsigned long, int, Kokkos::View<unsigned long const*, Kokkos::LayoutLeft, Kokkos::Device<Kokkos::Serial, Kokkos::HostSpace>, Kokkos::MemoryTraits<0u> >, Kokkos::View<unsigned long const*, Kokkos::LayoutLeft, Kokkos::Device<Kokkos::Serial, Kokkos::HostSpace>, Kokkos::MemoryTraits<0u> >, Kokkos::View<int const*, Kokkos::LayoutLeft, Kokkos::Device<Kokkos::Serial, Kokkos::HostSpace>, Kokkos::MemoryTraits<0u> >, Kokkos::View<int const*, Kokkos::LayoutLeft, Kokkos::Device<Kokkos::Serial, Kokkos::HostSpace>, Kokkos::MemoryTraits<0u> >, Kokkos::View<unsigned long*, Kokkos::LayoutLeft, Kokkos::Device<Kokkos::Serial, Kokkos::HostSpace>, Kokkos::MemoryTraits<0u> > >::ORDINAL_MAX'

Please confirm that the deduced template arguments are the intended template arguments when using GO=long long and LO = int.
@tcfisher tcfisher added the type: bug The primary issue is a bug in Trilinos code or tests label Dec 3, 2020
@jhux2
Copy link
Member

jhux2 commented Dec 4, 2020

@trilinos/tpetra

@jhux2 jhux2 added client: SPARC Issues related to or needed more specifically by the ATDM SPARC code pkg: Xpetra labels Dec 4, 2020
@jhux2
Copy link
Member

jhux2 commented Dec 4, 2020

@trilinos/xpetra

@brian-kelley
Copy link
Contributor

brian-kelley commented Dec 8, 2020 •
edited
Loading

@tcfisher Sorry I missed this notification. The missing symbol ORDINAL_MAX is a static constexpr member of a class, and I didn't know that (until C++17), you still need to give those an out-of-class definition like with normal static members. GCC and Clang let me do it anyway, but XL doesn't. I'll post a fix that doesn't involve static constexprs at all.

@jhux2
Copy link
Member

jhux2 commented Dec 8, 2020

@brian-kelley Thank you for looking at this.

brian-kelley added a commit to brian-kelley/Trilinos that referenced this issue Dec 8, 2020
@brian-kelley
KokkosKernels: remove static constexpr member (trilinos#8418)
e1daad5 
Remove static constexpr member variables from SpADD functors.
In official C++ pre 17, these need out-of-class definitions just like
non-constexpr static members, but I didn't add them here.
GCC/Clang let this work anyway, but XL goes by the
standard so this fixes building with XL.
@brian-kelley
Copy link
Contributor

OK, I thought I understood this but there are 2 other places in KokkosKernels that also use static constexpr variables, without a definition in a .cpp, but those didn't cause any errors on XL (I did replicate the linker error with ORDINAL_MAX).

brian-kelley added a commit to brian-kelley/Trilinos that referenced this issue Dec 9, 2020
@brian-kelley
KokkosKernels: remove static constexpr member (trilinos#8418)
fd6e399 
Remove static constexpr member variables from SpADD functors.
In official C++ pre 17, these need out-of-class definitions just like
non-constexpr static members, but I didn't add them here.
GCC/Clang let this work anyway, but XL goes by the
standard so this fixes building with XL.
This was referenced Dec 9, 2020
Merged
Merged
@brian-kelley
Copy link
Contributor

#8443 fixes this. I built with XL on vortex all tests for KokkosKernels, Tpetra, Ifpack2, MueLu and got no errors. A handful of tests failed but I don't think they're related to this.

trilinos-autotester added a commit that referenced this issue Dec 15, 2020
@trilinos-autotester
Merge Pull Request #8443 from brian-kelley/Trilinos/FixXLSpadd
0d1b458 
Automatically Merged using Trilinos Pull Request AutoTester
PR Title: KokkosKernels: remove static constexpr member (#8418)
PR Author: brian-kelley
jmgate pushed a commit to tcad-charon/Trilinos that referenced this issue Dec 15, 2020
Merge 'trilinos/Trilinos:develop' (0d1b458) into 'tcad-charon/Trilino…
7afaf40 
…s:develop' (e00327c).

* trilinos-develop:
  Panzer STK: Add a test to verify the ability to query the PerceptMesh member
  Panzer STK: Add functionality for keeping and for querying an STK_Interface's PerceptMesh member
  Framework: Reduce rate limit for stale job actionscript
  Updated ninja
  Panzer: fix race condition with launch blocking off in GED accessor.
  MueLu: applying an ugly fix in SaPFactory_kokkos constraint algo
  Updated to cmake 19 for Ross' cuda schedule fixes
  TrilinosCouplings: Fixing maxwell example
  Tpetra: Fix issue in `transferAndFillComplete`
  Tpetra: Add test for trilinos#8447
  forgot to add this fix to change the value in the P matrix
  Moving the Teko configuration to use the Tpetra default LO/GOs
  Add a test to MueLu_UnitTestsTpetra_kokkos that checks the SaP factory including the enforce constaint option.
  fix up some gold files
  fix up some gold files
  fix up some gold files
  fix up some gold files
  KokkosKernels: remove static constexpr member (trilinos#8418)
  fixed up an unused variable and a scalar traits comparison ... as well as updated a couple of gold files
  kokkos version of satisfy constraints for SaP
jmgate pushed a commit to tcad-charon/Trilinos that referenced this issue Dec 15, 2020
Merge 'trilinos/Trilinos:develop' (0d1b458) into 'tcad-charon/Trilino…
c7c52c5 
…s:develop' (e00327c).

* trilinos-develop:
  Panzer STK: Add a test to verify the ability to query the PerceptMesh member
  Panzer STK: Add functionality for keeping and for querying an STK_Interface's PerceptMesh member
  Framework: Reduce rate limit for stale job actionscript
  Updated ninja
  Panzer: fix race condition with launch blocking off in GED accessor.
  MueLu: applying an ugly fix in SaPFactory_kokkos constraint algo
  Updated to cmake 19 for Ross' cuda schedule fixes
  TrilinosCouplings: Fixing maxwell example
  Tpetra: Fix issue in `transferAndFillComplete`
  Tpetra: Add test for trilinos#8447
  forgot to add this fix to change the value in the P matrix
  Moving the Teko configuration to use the Tpetra default LO/GOs
  Add a test to MueLu_UnitTestsTpetra_kokkos that checks the SaP factory including the enforce constaint option.
  fix up some gold files
  fix up some gold files
  fix up some gold files
  fix up some gold files
  KokkosKernels: remove static constexpr member (trilinos#8418)
  fixed up an unused variable and a scalar traits comparison ... as well as updated a couple of gold files
  kokkos version of satisfy constraints for SaP
@kddevin
Copy link
Contributor

kddevin commented Jan 11, 2021

Appears to be fixed. Please re-open if not. Thanks for reporting, @tcfisher

@kddevin kddevin closed this as completed Jan 11, 2021
seamill pushed a commit to seamill/Trilinos that referenced this issue Feb 3, 2021
Merge 'trilinos/Trilinos:develop' (b253b63) into 'EM-Plasma/Trilinos:…
caa40dd 
…develop' (c6ac7b3).

* potential-trilinos-develop: (26 commits)
  Revert commit 75febde (trilinos#8471)
  Ifpack2: fix timers
  Panzer STK: Add a test to verify the ability to query the PerceptMesh member
  Panzer STK: Add functionality for keeping and for querying an STK_Interface's PerceptMesh member
  Automatic snapshot commit from tribits at 18f7981
  Automatic snapshot commit from tribits at 0591d05
  We don't need tweeks files for spack-rhel contributed (ATDV-406)
  MueLu: regionMG replacing Map with const Map where possible to avoid casting
  MueLu: applying an ugly fix in SaPFactory_kokkos constraint algo
  Bring back ride, remove serrano
  forgot to add this fix to change the value in the P matrix
  Moving the Teko configuration to use the Tpetra default LO/GOs
  Add a test to MueLu_UnitTestsTpetra_kokkos that checks the SaP factory including the enforce constaint option.
  fix up some gold files
  fix up some gold files
  fix up some gold files
  fix up some gold files
  KokkosKernels: remove static constexpr member (trilinos#8418)
  fixed up an unused variable and a scalar traits comparison ... as well as updated a couple of gold files
  kokkos version of satisfy constraints for SaP
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
client: SPARC Issues related to or needed more specifically by the ATDM SPARC code pkg: Tpetra pkg: Xpetra type: bug The primary issue is a bug in Trilinos code or tests
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@brian-kelley @kddevin @jhux2 @tcfisher