ECP-copa / Cabana

Performance-portable library for particle-based simulations
Other
197 stars 51 forks source link

Use Kokkos after its CMake overhaul #163

Closed dalg24 closed 4 years ago

dalg24 commented 4 years ago

@junghans I left out Travis CI for now

junghans commented 4 years ago

Can you modify https://github.com/ECP-copa/Cabana/blob/master/.travis.yml#L39 to test travis as well?

sfogerty commented 4 years ago

Let's remember to change the wiki build instructions when this goes through.

Building Kokkos' using its develop branch using CMake

codecov-io commented 4 years ago

Codecov Report

Merging #163 into master will not change coverage. The diff coverage is 12.5%.

Impacted file tree graph

@@          Coverage Diff           @@
##           master    #163   +/-   ##
======================================
  Coverage    47.6%   47.6%           
======================================
  Files          25      25           
  Lines        2692    2692           
======================================
  Hits         1283    1283           
  Misses       1409    1409
Flag Coverage Δ
#clang 47.6% <12.5%> (ø) :arrow_up:
Impacted Files Coverage Δ
core/example/longrange/ewald.h 0% <ø> (ø) :arrow_up:
core/src/Cabana_Slice.hpp 95.7% <ø> (ø) :arrow_up:
core/src/Cabana_DeepCopy.hpp 91.5% <ø> (ø) :arrow_up:
core/example/longrange/particles.cpp 0% <0%> (ø) :arrow_up:
core/example/longrange/spme_example.cpp 0% <0%> (ø) :arrow_up:
core/example/longrange/spme.cpp 0% <0%> (ø) :arrow_up:
core/example/longrange/ewald.cpp 0% <0%> (ø) :arrow_up:
core/example/longrange/ewaldsum_example.cpp 0% <0%> (ø) :arrow_up:
core/src/Cabana_VerletList.hpp 99.5% <100%> (ø) :arrow_up:
core/src/Cabana_Parallel.hpp 100% <100%> (ø) :arrow_up:
... and 16 more

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update f625a4a...76e8f16. Read the comment docs.

junghans commented 4 years ago

Hmm, I have no idea by AMPI segfaults with 4 tasks, @evan-charmworks any idea?

matthiasdiener commented 4 years ago

@junghans: The segfault is actually an unhandled C++ exception in the application code. Charm++/AMPI do not use exceptions, but we have a default exception handler that aborts on all uncaught exceptions: https://github.com/UIUC-PPL/charm/commit/becc7731feef74b4001fbe31a85658d054411fb6

All the exceptions seem to be thrown by Kokkos::SharedAllocationRecord::decrement: https://github.com/kokkos/kokkos/blob/master/core/src/impl/Kokkos_SharedAlloc.cpp

junghans commented 4 years ago

@dalg24 now we are back in kokkos space for the exception.

dalg24 commented 4 years ago

@matthiasdiener Thanks for looking into it. I installed Charm++ this morning and wasn't able to reproduce the error. Do you remember what line was throwing? It really puzzles me because there is no AMPI specific code on Cabana as far as I can tell and no exception is thrown with other MPI implementations.

matthiasdiener commented 4 years ago

@dalg24: I haven't build this myself, but this is what I understood from the build logs in Travis. I don't know which of the lines in Kokkos::SharedAllocationRecord::decrement actually throws the exceptions (I think there are 3 lines in that function where exceptions could be thrown). Do you have a script to build Kokkos/Cabana?

dalg24 commented 4 years ago

@matthiasdiener For Kokkos configure with

<kokkos_source_dir>/generate_makefile.bash --with-options=disable_deprecated_code --prefix=<kokkos_install_prefix>

For Cabana


cmake -DMPIEXEC=ampirun -DMPI_CXX_COMPILER=ampicxx -DCabana_ENABLE_MPI=ON -DCabana_ENABLE_TESTING=ON -DCMAKE_INSTALL_PREFIX=<kokkos_install_prefix> -DCabana_ENABLE_Serial=ON <cabana_source_dir>
sslattery commented 4 years ago

Retest this once #171 is merged

sslattery commented 4 years ago

If we want to depend on a Kokkos release then the merge of this PR will be blocked by the release of Kokkos 3.0

junghans commented 4 years ago

@dalg24 are we ready to pull the trigger here?