Remote space support #1886

crtrott · 2018-11-06T22:09:51Z

No description provided.

ndellingwood · 2018-11-06T22:29:35Z

core/src/Kokkos_View.hpp

@@ -542,7 +552,7 @@ class View : public ViewTraits< DataType , Properties ... > {

 private:

-  typedef Kokkos::Impl::ViewMapping< traits , void > map_type ;
+  typedef Kokkos::Impl::ViewMapping< traits , typename traits::specialize > map_type ;


@crtrott I think this change will affect Sacado, the ViewMapping there expects void, but this change will result (in Sacado) in the second template parameter being some specialize tag (e.g. ViewSpecializeSacadoFad etc). The change there seems like it should be easy, but we should check with Eric in case of overlooking something.

Yeah, look at my new commit which fixes the internal aggregates test which mimics what Sacado does. It actually makes the code cleaner and I was able to delete parts of the specialization.

The only potential issue I see is that Sacado currently has several partial specializations of ViewMapping<> for a given view specialization tag that are differentiated through enable_if<>, to support assignments between different view specializations (see, e.g., the ViewMapping<> specializations here.) This is to allow, e.g, View<double...> = View<Fad...> etc. Will that still work?

@etphipp it may be as simple as replacing the final explicit void template parameter in ViewMapping typedefs with the appropriate specialization tag except for the case you mentioned above, e.g. leave as void for the DstType in this case:
line 1365 and in matching places for the other specialized ViewMapping types.

Also making sure to change the default void return type of the enable_ifs used in the ViewMapping specialization declarations to return the proper specialize tag.

crtrott · 2018-11-07T00:18:56Z

ModuleCmd_Load.c(213):ERROR:105: Unable to locate a modulefile for 'git'
Running on machine: apollo
Repository Status:  f0b89550ca7f50d9115d12c035f9cc3ab784eea9 Fix ViewArray aggregate type


Going to test compilers:  gcc/4.8.4 gcc/5.3.0 intel/16.0.1 clang/3.9.0 clang/6.0 cuda/9.1
Testing compiler gcc/4.8.4
  Starting job gcc-4.8.4-OpenMP-release
  PASSED gcc-4.8.4-OpenMP-release
Testing compiler gcc/5.3.0
  Starting job gcc-4.8.4-Pthread-release
  PASSED gcc-4.8.4-Pthread-release
Testing compiler intel/16.0.1
  Starting job gcc-5.3.0-Serial-release
  PASSED gcc-5.3.0-Serial-release
Testing compiler clang/3.9.0
  Starting job intel-16.0.1-OpenMP-release
  PASSED intel-16.0.1-OpenMP-release
Testing compiler clang/6.0
  Starting job clang-3.9.0-Pthread_Serial-release
  PASSED clang-3.9.0-Pthread_Serial-release
  Starting job clang-6.0-Cuda_Pthread-release
  PASSED clang-6.0-Cuda_Pthread-release
Testing compiler cuda/9.1
  Starting job clang-6.0-OpenMP-release
  PASSED clang-6.0-OpenMP-release
  Starting job cuda-9.1-Cuda_OpenMP-release
  PASSED cuda-9.1-Cuda_OpenMP-release
#######################################################
PASSED TESTS
#######################################################
clang-3.9.0-Pthread_Serial-release build_time=128 run_time=440
clang-6.0-Cuda_Pthread-release build_time=249 run_time=314
clang-6.0-OpenMP-release build_time=125 run_time=106
cuda-9.1-Cuda_OpenMP-release build_time=307 run_time=216
gcc-4.8.4-OpenMP-release build_time=103 run_time=95
gcc-4.8.4-Pthread-release build_time=97 run_time=319
gcc-5.3.0-Serial-release build_time=118 run_time=165
intel-16.0.1-OpenMP-release build_time=331 run_time=141

crtrott · 2018-11-07T00:28:52Z

@etphipp Eric please check this pull request since it will affect Sacado. But I believe it makes it actually ever so slightly clearer how to specialize these things.

View: update various ctors

ndellingwood · 2018-11-08T01:29:28Z

@crtrott I pushed some changes needed for DynRankViews, so this will need re-spot-checked.

@etphipp I attached patches for sacado and stokhos with fixes for this PR and also created a new kokkos-promotion branch in Trilinos to accumulate changes as the come (this PR will break Trilinos so changes will need to accumulate for the next promotion). Are you fine with the changes?

I tested (sym-linking to this branch of kokkos) with a Cuda_Serial build on Waterman. There were no Sacado test failures and one Stokhos test failure that was failing with/without Kokkos changes during integration testing:

The following tests FAILED:
	 61 - Stokhos_KokkosArrayKernelsUnitTest_Cuda_MPI_1 (Failed)

Specific subtest:

61: The following tests FAILED:
61:     7. Kokkos_SG_SpMv_double_Cuda_CrsProductTensor_UnitTest ...

sacado-kokkos1886.patch.txt

stokhos-kokkos1886.patch.txt

Here was my testing setup on waterman:

Configuration:

export TRILINOS_DIR=${PWD}/../..

# Load modules
module purge
source ${TRILINOS_DIR}/cmake/std/atdm/load-env.sh cuda-9.2-opt
module swap cmake/3.6.2 cmake/3.12.3
#source $TRILINOS_DIR/cmake/std/atdm/load-env.sh cuda-9.2-debug-Kepler37

# Packages
PACKAGE1=Sacado
PACKAGE1=Stokhos

# Configure
cmake \
 -GNinja \
 -DTrilinos_CONFIGURE_OPTIONS_FILE:STRING=cmake/std/atdm/ATDMDevEnv.cmake \
 -DTrilinos_ENABLE_TESTS=ON \
  -DTrilinos_ENABLE_${PACKAGE1}=ON \
  -DTrilinos_ENABLE_${PACKAGE2}=ON \
  -DKOKKOS_ENABLE_DEPRECATED_CODE=ON \
  -DKokkos_SOURCE_DIR_OVERRIDE:STRING=kokkos \
  -DKokkosKernels_SOURCE_DIR_OVERRIDE:STRING=kokkos-kernels \
$TRILINOS_DIR

Source the configuration script, build the code (on or off node), then allocate a node to run the

bsub -Is -n 16 -q rhel7W bash

ctest

Changes to coordinate with kokkos/kokkos#1886. Various ViewMapping partial specializations now require the final template parameter to be the specialize tag, rather than explicit void. These changes ensure Sacado and Stokhos compatibility with the Kokkos changes. Test with Cuda_Serial build on waterman with cuda/9.2+gcc/7.2 Changes to be committed: modified: packages/sacado/src/KokkosExp_View_Fad.hpp modified: packages/sacado/src/KokkosExp_View_Fad_Contiguous.hpp modified: packages/sacado/src/Kokkos_DynRankView_Fad.hpp modified: packages/sacado/src/Kokkos_DynRankView_Fad_Contiguous.hpp modified: packages/sacado/test/UnitTests/Fad_KokkosTests_Cuda.cpp modified: packages/stokhos/src/sacado/kokkos/pce/KokkosExp_View_UQ_PCE_Contiguous.hpp modified: packages/stokhos/src/sacado/kokkos/vector/KokkosExp_View_MP_Vector_Contiguous.hpp

mhoemmen · 2018-11-08T18:19:53Z

Just curious -- does this PR have anything to do with "remote space support"? It looks like it's all about View specialization.

ndellingwood · 2018-11-08T20:45:38Z

@crtrott kokkos-dev spot-check passes:

Running on machine: sems
Repository Status:  18ab598c65714bae1f0df64e3c329c8703f943b0 DynRanView,View: update ViewMapping to use specialize tag 


Going to test compilers:  gcc/5.3.0 gcc/7.3.0 intel/17.0.1 clang/4.0.1 cuda/8.0.44
Testing compiler gcc/5.3.0
Testing compiler gcc/7.3.0
  Starting job gcc-5.3.0-OpenMP-release
  PASSED gcc-5.3.0-OpenMP-release
Testing compiler intel/17.0.1
  Starting job gcc-7.3.0-Serial-release
  PASSED gcc-7.3.0-Serial-release
Testing compiler clang/4.0.1
  Starting job intel-17.0.1-OpenMP-release
  PASSED intel-17.0.1-OpenMP-release
Testing compiler cuda/8.0.44
  Starting job clang-4.0.1-Pthread_Serial-release
  PASSED clang-4.0.1-Pthread_Serial-release
  Starting job cuda-8.0.44-Cuda_OpenMP-release
  PASSED cuda-8.0.44-Cuda_OpenMP-release
#######################################################
PASSED TESTS
#######################################################
clang-4.0.1-Pthread_Serial-release build_time=184 run_time=388
cuda-8.0.44-Cuda_OpenMP-release build_time=552 run_time=628
gcc-5.3.0-OpenMP-release build_time=176 run_time=94
gcc-7.3.0-Serial-release build_time=149 run_time=164
intel-17.0.1-OpenMP-release build_time=438 run_time=150

ndellingwood · 2018-11-08T20:46:15Z

@mhoemmen I think the changes are needed for a separate branch or repo where Christian is working on the remote space support.

etphipp · 2018-11-13T20:26:05Z

I added a PR to trilinos that fixes the test failure,

trilinos/Trilinos#3867

which is unrelated to this PR here.

ndellingwood · 2018-11-28T18:39:21Z

@crtrott This should be ready to merge, I created a new kokkos-promotion branch of Trilinos to track the sacado and stokhos fixes necessary to support this change.

crtrott added 3 commits May 8, 2018 16:20

Allow specialisation of Mapping object

8d77ed4

Merge branch 'develop' into remote-space-support

c05c48e

Merge branch 'develop' into remote-space-support

4497c21

ndellingwood reviewed Nov 6, 2018

View reviewed changes

Fix ViewArray aggregate type

f0b8955

DynRanView,View: update ViewMapping to use specialize tag

18ab598

View: update various ctors

ndellingwood approved these changes Nov 8, 2018

View reviewed changes

etphipp mentioned this pull request Nov 13, 2018

Stokhos: Fixes for volta. trilinos/Trilinos#3867

Merged

crtrott merged commit 27ae89f into develop Nov 28, 2018

crtrott deleted the remote-space-support branch November 28, 2018 18:49

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Remote space support #1886

Remote space support #1886

crtrott commented Nov 6, 2018

ndellingwood Nov 6, 2018

crtrott Nov 6, 2018

etphipp Nov 7, 2018

ndellingwood Nov 7, 2018

crtrott commented Nov 7, 2018

crtrott commented Nov 7, 2018

ndellingwood commented Nov 8, 2018

mhoemmen commented Nov 8, 2018

ndellingwood commented Nov 8, 2018

ndellingwood commented Nov 8, 2018

etphipp commented Nov 13, 2018

ndellingwood commented Nov 28, 2018

Remote space support #1886

Remote space support #1886

Conversation

crtrott commented Nov 6, 2018

ndellingwood Nov 6, 2018

Choose a reason for hiding this comment

crtrott Nov 6, 2018

Choose a reason for hiding this comment

etphipp Nov 7, 2018

Choose a reason for hiding this comment

ndellingwood Nov 7, 2018

Choose a reason for hiding this comment

crtrott commented Nov 7, 2018

crtrott commented Nov 7, 2018

ndellingwood commented Nov 8, 2018

mhoemmen commented Nov 8, 2018

ndellingwood commented Nov 8, 2018

ndellingwood commented Nov 8, 2018

etphipp commented Nov 13, 2018

ndellingwood commented Nov 28, 2018