Feature/kokkos #188

ajpowelsnl · 2021-09-16T18:26:31Z

Summary

@davidbeckingsale and @rhornung67 have been our main points of contact and collaboration in this work

The proposed code implements RAJAPerf Suite (RPS) - based performance testing for Kokkos.

Main new code and changes include:

Bringing Kokkos in as a RPS (tpl) submodule
Kokkos implementations (with Kokkos_Lambda as a kernel variant) of the basic, stream, lcals , apps and algorithm kernel groups (polybench will be completed in FY22)
Changes to the build system / cmake
Significant changes to the infrastructure for registering new kernel groups and new kernels:

common/Executor.hpp: kernelID registerKernel(std::string, KernelBase*);
common/Executor.cpp: exec->registerKernel(groupName, kernel);
RAJAPerfSuiteDriver.cpp: //executor.registerKernel

Configure Kokkos build and runtime in RPS:

cmake
-DENABLE_KOKKOS=ON
-DENABLE_OPENMP=ON
-DCMAKE_BUILD_TYPE=Debug .. \

Please do not hesitate to contact @ajpowelsnl or @DavidPoliakoff for any questions and / or further discussion.

Conflicts: CMakeLists.txt src/basic/CMakeLists.txt

Conflicts: .gitmodules src/common/Executor.cpp src/common/RAJAPerfSuite.cpp src/common/RunParams.cpp tpl/RAJA

…feature/kokkos

ajpowelsnl · 2021-10-26T19:42:42Z

@ajpowelsnl did you branch from an older version of the persuite. I was interested in looking at the kokkos version of the MAT_MAT_SHARED kernel, but don't see it.

Hi @artv3 -- Thanks for the question. I started working on the project late last year (first commit was Nov. 16, 2020), so it probably is an "older" version of the Perf Suite. I did not work on anything called "MAT_MAT_SHARED." Instead, I provided Kokkos translations of RAJAPerf Suite kernels (that existed around that date). Does that make sense?

ajpowelsnl · 2021-10-26T20:40:45Z

@davidbeckingsale , @DavidPoliakoff -- I'm also removing or suppressing this debugging output in this commit:

Running specified kernels and variants...
Value is 3.14159
Value is 3.14159
Value is 3.14159
Value is 3.14159
Value is 3.14159
Value is 3.14159
Value is 3.14159
FIX ME STREAM DOT -- GET DATA FROM VIEWS

artv3 · 2021-10-27T17:05:29Z

@ajpowelsnl did you branch from an older version of the persuite. I was interested in looking at the kokkos version of the MAT_MAT_SHARED kernel, but don't see it.

Hi @artv3 -- Thanks for the question. I started working on the project late last year (first commit was Nov. 16, 2020), so it probably is an "older" version of the Perf Suite. I did not work on anything called "MAT_MAT_SHARED." Instead, I provided Kokkos translations of RAJAPerf Suite kernels (that existed around that date). Does that make sense?

Yes, do you plan to add Kokkos kernels for newer kernels in RAJAPerf?

ajpowelsnl · 2021-10-27T17:54:22Z

@ajpowelsnl did you branch from an older version of the persuite. I was interested in looking at the kokkos version of the MAT_MAT_SHARED kernel, but don't see it.

Hi @artv3 -- Thanks for the question. I started working on the project late last year (first commit was Nov. 16, 2020), so it probably is an "older" version of the Perf Suite. I did not work on anything called "MAT_MAT_SHARED." Instead, I provided Kokkos translations of RAJAPerf Suite kernels (that existed around that date). Does that make sense?

Yes, do you plan to add Kokkos kernels for newer kernels in RAJAPerf?

Morning @artv3 -- Thanks again for your question, and your interest. @DavidPoliakoff, @davidbeckingsale and I have briefly discussed providing Kokkos translations for the new kernels , but that area of work has not been discussed yet with our project leads / folks who control the purse strings. Do you have a particular interest in Kokkos versions of the new kernels? If so, then it might help me strengthen the argument for doing the Kokkos translations of the new kernels. Does it benefit both groups to understand performance behavior over time?

artv3 · 2021-10-27T18:21:00Z

@ajpowelsnl did you branch from an older version of the persuite. I was interested in looking at the kokkos version of the MAT_MAT_SHARED kernel, but don't see it.

Hi @artv3 -- Thanks for the question. I started working on the project late last year (first commit was Nov. 16, 2020), so it probably is an "older" version of the Perf Suite. I did not work on anything called "MAT_MAT_SHARED." Instead, I provided Kokkos translations of RAJAPerf Suite kernels (that existed around that date). Does that make sense?

Yes, do you plan to add Kokkos kernels for newer kernels in RAJAPerf?

Morning @artv3 -- Thanks again for your question, and your interest. @DavidPoliakoff, @davidbeckingsale and I have briefly discussed providing Kokkos translations for the new kernels , but that area of work has not been discussed yet with our project leads / folks who control the purse strings. Do you have a particular interest in Kokkos versions of the new kernels? If so, then it might help me strengthen the argument for doing the Kokkos translations of the new kernels. Does it benefit both groups to understand performance behavior over time?

Yes absolutely! Those kernels express hierarchical parallelism and for our applications we have found that by taking advantage of the memory hierachy on the GPU (using shared memory [CUDA/HIP] or device local memory [SYCL]) we can really improve kernel performance. Recognizing that I always wonder how do you expose these device feature features in a portable and friendly way across programing models and of course minimize abstraction layerover head. As new programming models come online how do we maintain these features?

Fixed timing code

CMakeLists.txt

davidbeckingsale · 2021-11-11T20:00:23Z

CMakeLists.txt

  list(APPEND RAJA_PERFSUITE_DEPENDS hip)
 endif()

 set(RAJAPERF_BUILD_SYSTYPE $ENV{SYS_TYPE})
 set(RAJAPERF_BUILD_HOST $ENV{HOSTNAME})

 if (ENABLE_CUDA)
-  set(CMAKE_CUDA_STANDARD 11)
+  if (ENABLE_CUDA AND ENABLE_KOKKOS)


The ENABLE_CUDA here is redundant (since we are already inside an if (ENABLE_CUDA) block.

CMakeLists.txt

davidbeckingsale · 2021-11-11T20:01:49Z

CMakeLists.txt

+    #set_target_properties(kokkoscore kokkoscontainers PROPERTIES LANGUAGE CUDA)
+  endif()
+  get_property(KOKKOS_INCLUDE_DIRS DIRECTORY tpl/kokkos PROPERTY INCLUDE_DIRECTORIES)
+  include_directories(${KOKKOS_INCLUDE_DIRS})


Is this required? I think that having kokkos in the RAJA_PERFSUITE_DEPENDS should be enough.

davidbeckingsale · 2021-11-11T20:02:48Z

src/CMakeLists.txt

+  lcals-kokkos/INT_PREDICT-Kokkos.cpp
+  lcals-kokkos/PLANCKIAN-Kokkos.cpp
+  lcals-kokkos/TRIDIAG_ELIM-Kokkos.cpp
+  #polybench/POLYBENCH_2MM.cpp


Why are these all commented?

The Polybench kernels are commented because we do not yet have Kokkos translations for them. I believe the situation calls for additional logic to deal with the relative immaturity of the Kokkos design. Does this make sense? It's a choice (commenting) David P and I made in the course of prototyping.

davidbeckingsale · 2021-11-11T20:03:45Z

src/RAJAPerfSuiteDriver.cpp

+  //rajaperf::RunParams params(argc, argv);
+  //executor.registerGroup("Sparse");
+
+  //executor.registerKernel("Sparse", rajaperf::make_kernel_base(


Commented code?

Deleted all debugging code.

davidbeckingsale · 2021-11-11T20:04:37Z

src/apps/WIP-COUPLE.cpp

@@ -20,184 +20,175 @@ namespace rajaperf
 namespace apps
 {

+//
+//COUPLE::COUPLE(const RunParams& params)


Lots more commented code?

…r.cpp, Executor.hpp

davidbeckingsale · 2021-12-16T13:07:58Z

src/CMakeLists.txt

 add_subdirectory(apps)
+add_subdirectory(apps-kokkos)


I think these should be guarded with ENABLE_KOKKOS

I think these should be guarded with ENABLE_KOKKOS

Hi David, if I am understanding your suggestion, guarding all things Kokkos in this file will require significant reorganization of this file. Is that what you would like for me to do, i.e., cleanly separate RAJAPerf Suite content from Kokkos content?

davidbeckingsale · 2021-12-16T13:08:11Z

src/CMakeLists.txt

 add_subdirectory(common)
+
+if(NOT INFRASTRUCTURE_ONLY)


Where does the INFRASTRUCTRUE_ONLY option come from?

Where does the INFRASTRUCTRUE_ONLY option come from?

Hi David -- Taking a step back to explain the "big picture", remember that we are using RAJAPerf Suite (RPS) in two distinct manners: 1) Provide Kokkos translations of RPS kernels that can be run along side native RPS kernels, and 2) Using RPS as the infrastructure driver for Kokkos Kernels and Kokkos performance tests that we have written. To be clear, this second case has nothing to do with RPS kernels & their Kokkos translations. This second case is tied to the "INFRASTRUCTURE_ONLY" option. Does that make sense?

The why makes sense, I meant that INFRASTRUCTURE_ONLY should be explicitly defined as a CMake option (https://cmake.org/cmake/help/latest/command/option.html) with a default value of False, that you can override when you are using the perfsuite as the infrastructure driver.

davidbeckingsale · 2021-12-16T13:08:52Z

src/CMakeLists.txt

  DEPENDS_ON ${RAJA_PERFSUITE_DEPENDS}
 )

 else()
+	if(NOT INFRASTRUCTURE_ONLY)


This looks unnecessary.

Hi David -- Please see my comment above on this point (i.e., there are two distinct scenarios in which we're using RAJAPerf Suite). Would it be possible for us to meet, and work through at least some / most of your suggested changes? I just want to make sure I'm doing what you want me to do for the merge.

ajpowelsnl · 2022-01-05T17:48:26Z

src/common/Executor.cpp

@@ -158,15 +165,18 @@ void Executor::setupSuite()
    run_params.setInvalidExcludeFeatureInput(invalid);

    //


TODO: @rhornung67 , @davidbeckingsale , there is functionality we have not yet implemented in Kokkos, but that is important for RAJAPerf Suite. Starting at line 167, and ending at ~ line 342 in Executor.cpp, we have commented this functionality. Rich, David B. suggested that I work with you on how to handle integrating this (currently) commented functionality with Kokkos design (i.e., dynamic addition of new kernels and kernel groups).

ajpowelsnl · 2022-01-05T17:55:12Z

src/common/RunParams.cpp

    str << std::endl;
  }  // loop over features
  str.flush();
 }
-
+// TODO for Kokkos Team:  Commenting function body, because this infrastructure 


TODO: @rhornung67 , @davidbeckingsale - commented lines here, 654-674, also represent important functionality for RAJAPerf Suite that has not yet been implemented for Kokkos. Would you guys please help me work through integrating these bits?

crtrott · 2022-01-11T23:57:11Z

Hey guys, I didn't yet read all the comments but I think we should try and split the infrastructure changes from the Kokkos kernel variant changes.

So essentially something like this:
PR1: Infrastructure changes (how to add Kernel groups etc. + let perf suite serve as a perf testing infrastructure in other projects [i.e. Kokkos Core/Kokkos Kernels])
PR2: Add optional Kokkos dependency with all the kernels variants.

@davidbeckingsale what do you think about this?

Note that PR2 are not really dependent on PR1 so we could also do those two first and then tackle the infrastructure change. We could also split PR2 into doing kernel groups individually I guess?

davidbeckingsale · 2022-01-12T18:41:04Z

@davidbeckingsale what do you think about this?

I agree that splitting the changes up into two distinct pieces would be best, and I suggested that approach before. I think the two main sticking points on this PR right now are 1) ensuring that the infrastructure changes maintain all the current capability, and 2) deciding how to handle cases where a variant is "missing".

ajpowelsnl · 2022-01-12T18:46:59Z

@davidbeckingsale what do you think about this?

I agree that splitting the changes up into two distinct pieces would be best, and I suggested that approach before. I think the two main sticking points on this PR right now are 1) ensuring that the infrastructure changes maintain all the current capability, and 2) deciding how to handle cases where a variant is "missing".

Hi David & Christian -- Many thanks for responding. As for sticking point 2, in the most recent push, I did stub in Kokkos variants for the polybench kernels, somewhat alleviating that issue. But we are still left with sticking point 1. I will work with Christian on this, and see if we can come up with a satisfactory solution that preserves current infrastructure capability.

DavidPoliakoff and others added 30 commits January 27, 2020 12:25

First pass at Kokkos in the build system

792224f

First example

9456d76

Merge branch 'develop' into feature/kokkos

4e4c2dd

Reworking to split out the Kokkos implementations

19afb97

Added in the Kokkos implementation file (whoops)

d658a4d

Moved Kokkos files into their own directory

f7631d1

Working state, can now just stamp out kernels

0f74831

Fixed up the sequential Kokkos version

c70a7ca

WIP: CUDA

b52d974

Pulled latest Kokkos

37c2897

Functional DAXPY

ae5007a

Corrected name

7f707f9

Update C++ standard

88f98f9

Changes for a minimal build

8b12325

Kokkos IF_QUAD basic kernels

8b828f5

Merge branch 'develop' into temp_kokkos_kernels

529746d

Conflicts: CMakeLists.txt src/basic/CMakeLists.txt

infrastructure changes and Kokkos_ATOMIC_PI

cfda905

basic-kokkos IF_QUAD and ATOMIC_PI kernels

b227e70

Adding piles o' files

b3ef7f5

Removed extra files

104ac3d

Rename misnamed files

c9cf2a7

Infrastructure fixes

fddf904

Small changes to enable correct output

7a37806

first implementation of a "kokkos-mechanics" test in RajaPerfSuite

7aa5d8d

Added stream-add with Kokkos Views

6858ba3

Adding basic kernels for Kokkos performance testing

3f8acf9

csv_xml.py: parser for RAJAPerf-timing.csv to xml

8bac825

csv_xml.py: raw xml dump of csv results

c1193dc

csv_xml.py: updates to raw xml output

9d0501c

KokkosSeq: Kokkos-i-fied basic kernels

5ed43c0

ajpowelsnl added 6 commits October 6, 2021 10:46

VOL3D-Kokkos.cpp: Fix #ifdefs, #endifs

8e56aa4

HALOEXCHANGE-Kokkos.cpp: Kokkos translation

ff48c3a

FIR-Kokkos.cpp: Wrapping coeff array in View

621e382

Merge remote-tracking branch 'upstream/develop' into feature/kokkos

f19da0a

Conflicts: .gitmodules src/common/Executor.cpp src/common/RAJAPerfSuite.cpp src/common/RunParams.cpp tpl/RAJA

Merge branch 'feature/kokkos' of github.com:ajpowelsnl/rajaperf into …

29c3460

…feature/kokkos

commenting features not integrated into Kokkos testing

038c34e

Addressing David B.'s first batch of PR Comments

f9d829d

ajpowelsnl and others added 7 commits November 2, 2021 12:18

Fix some compiler warnings

ec0421b

Merge 1: RPS + RPS AS INFRASTRUCTURE

520357d

Merge 2: RPS + INFRASTRUCTURE

c064d50

Fixing test reps and problem size numbers

29dfbfd

Fixed timing code

393504b

Merge pull request #1 from DavidPoliakoff/feature/kokkos

201d7ad

Fixed timing code

rm name passed to pop.region()

b7de26a

davidbeckingsale requested changes Nov 11, 2021

View reviewed changes

ajpowelsnl added 2 commits November 29, 2021 13:05

First major clean up for David B.: does not include commented Executo…

3dc1751

…r.cpp, Executor.hpp

First pass clean up of Executor files

18cca87

davidbeckingsale requested changes Dec 16, 2021

View reviewed changes

Addressing PR comments from David Beckingsale & polybench-kokkos stubs

487bce9

ajpowelsnl commented Jan 5, 2022

View reviewed changes

ajpowelsnl mentioned this pull request Jan 13, 2022

Basic kokkos #206

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feature/kokkos #188

Feature/kokkos #188

ajpowelsnl commented Sep 16, 2021

ajpowelsnl commented Oct 26, 2021

ajpowelsnl commented Oct 26, 2021 •

edited

Loading

artv3 commented Oct 27, 2021

ajpowelsnl commented Oct 27, 2021

artv3 commented Oct 27, 2021 •

edited

Loading

davidbeckingsale Nov 11, 2021

davidbeckingsale Nov 11, 2021

davidbeckingsale Nov 11, 2021

ajpowelsnl Dec 21, 2021 •

edited

Loading

davidbeckingsale Nov 11, 2021

ajpowelsnl Dec 21, 2021

davidbeckingsale Nov 11, 2021

davidbeckingsale Dec 16, 2021

ajpowelsnl Dec 20, 2021

davidbeckingsale Dec 16, 2021

ajpowelsnl Dec 20, 2021

davidbeckingsale Dec 20, 2021

davidbeckingsale Dec 16, 2021

ajpowelsnl Dec 20, 2021

ajpowelsnl Jan 5, 2022

ajpowelsnl Jan 5, 2022

crtrott commented Jan 11, 2022

davidbeckingsale commented Jan 12, 2022

ajpowelsnl commented Jan 12, 2022

		@@ -158,15 +165,18 @@ void Executor::setupSuite()
		run_params.setInvalidExcludeFeatureInput(invalid);

		//

		add_subdirectory(apps)
		add_subdirectory(apps-kokkos)

Feature/kokkos #188

Are you sure you want to change the base?

Feature/kokkos #188

Conversation

ajpowelsnl commented Sep 16, 2021

Summary

Configure Kokkos build and runtime in RPS:

ajpowelsnl commented Oct 26, 2021

ajpowelsnl commented Oct 26, 2021 • edited Loading

artv3 commented Oct 27, 2021

ajpowelsnl commented Oct 27, 2021

artv3 commented Oct 27, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ajpowelsnl Dec 21, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

crtrott commented Jan 11, 2022

davidbeckingsale commented Jan 12, 2022

ajpowelsnl commented Jan 12, 2022

ajpowelsnl commented Oct 26, 2021 •

edited

Loading

artv3 commented Oct 27, 2021 •

edited

Loading

ajpowelsnl Dec 21, 2021 •

edited

Loading