Use new kokkos functionality #1884

kruger · 2017-10-20T19:09:21Z

cmake/ProjectCompilerPostConfig.cmake now invokes the kokkos
build system to get additional compiler information that is useful for
advanced architectures. This enables the useful information that Kokkos
calculates can be more widely available.

cmake/ProjectCompilerPostConfig.cmake now invokes the kokkos build system to get additional compiler information that is useful for advanced architectures. This enables the useful information that Kokkos calculates can be more widely available.

mhoemmen · 2017-10-22T20:27:15Z

Could you please redo the pull request to be merged against Trilinos:develop? Thanks!

tjfulle · 2017-10-22T20:36:42Z

I believe, instead of closing and redoing the PR, it can be edited to be against develop. Just hit the edit button near the title

mhoemmen

Hope I'm not intruding, but I wanted to make sure to point out some possible build issues and make suggestions about improving the documentation. Thanks!

mhoemmen · 2017-10-22T20:28:41Z

cmake/ProjectCompilerPostConfig.cmake

+
+  #------------ GENERATE HEADER AND SOURCE FILES -------------------------------
+  execute_process(
+    COMMAND ${KOKKOS_SETTINGS} make -f ${KOKKOS_SRC_PATH}/core/src/Makefile build-makefile-cmake-kokkos


Would this break the Windows build, or is CMake smart enough to figure out what make means?

Kokkos doesn't build with MS Visual C++ yet as far as I know, but it may build on Windows in Visual Studio with the Intel compiler.

This should work with Cygwin or the Ubuntu add-on but it would require testing.

@kruger The projects in question use Visual Studio, not Cygwin or the Ubuntu add-on.

As far as I can tell, this would be the first bit of CMake code in Trilinos that invokes make directly via execute_process. Does TriBITS require that make be in $PATH?

Also, would this break the ninja back-end?

@bartlettroscoe Can you comment on make requirements?

@mhoemmen The execute_process is done at configure time and is just like invoking a script to calculate build parameters. It's not using make for the build itself.

I'll let @bartlettroscoe give you further direction. I'm just pointing out potential issues.

@kruger and @mhoemmen,

For some reason, I did not see this PR or the comments from earlier in the month.

This usage of make has nothing to do with the generator used by for the build of Trilinos objects. It should work with both the Ninja and Makefile generators (and any other CMake backends). But yet, it will require that make be in the path. But to be more flexible, perhaps we should use FIND_PROGRAM() to to find make and then let the user point to a different make.

mhoemmen · 2017-10-22T20:36:01Z

doc/build_ref/TrilinosBuildReferenceTemplate.rst

+Example: Configuring with Kokkos and advanced back-ends
+--------------------------------------------------------
+
+Kokkos serves as a basis for many advanced trilinos packages, especially those


This doesn't say what Kokkos is or why users might want it. I'm also not a fan of words like "advanced" that don't have a clear meaning. It's like saying "fastest" or "only the latest." Kokkos will work fine on older hardware too.

How about this: "Kokkos (https://github.com/kokkos/kokkos) is a C++ implementation of a cross-platform shared-memory parallel programming model. Many Trilinos packages, and other stand-alone applications, use it to implement parallel algorithms. Kokkos has two different build systems: Trilinos' CMake-based build system, and its own separate build system."

Regarding "...and this build system has more options than that supported by Trilinos by default..." -- isn't the point of this pull request to fix that?

I took your verbiage. Thanks!

My goal was to explain why a user would have something like -DTPL_ENABLE_CUDA=True -DKOKKOS_ENABLE_CUDA_LDG_INTRINSIC=True. Why TPL for one and not the other?

I think with your fixes it is sufficient.

mhoemmen · 2017-10-22T20:39:45Z

doc/build_ref/TrilinosBuildReferenceTemplate.rst

+  +----------------------------+----------------------------------------------+
+  | Debug builds               | KOKKOS_DEBUG                                 |
+  +----------------------------+----------------------------------------------+
+  | Devices Options                                                           |


Grammar fix: "Device options" (note capitalization change, for consistency with the other column headers).

mhoemmen · 2017-10-22T20:41:04Z

doc/build_ref/TrilinosBuildReferenceTemplate.rst

+  +----------------------------+----------------------------------------------+
+  | CUDA Options                                                              |
+  +----------------------------+----------------------------------------------+
+  | Enable CUDA LDG            | KOKKOS_ENABLE_CUDA_LDG_INTRINSIC             |


Is the point of this list to document the available options, or just to list them? I ask because you explain the UVM acronym below, but not the LDG acronym above.

I am copying documentation from elsewhere in Kokkos so I was giving as much info as I have.

I'll add (global memory load)

trilinos-autotester · 2017-11-20T14:52:14Z

Status Flag 'Pre-Test Inspection' - - This Pull Request Requires Inspection... The code must be inspected by a member of the Team before Testing/Merging
THE LAST COMMIT TO THIS PULL REQUEST HAS NOT BEEN REVIEWED YET!

bartlettroscoe · 2017-11-20T19:27:20Z

@kruger,

Can you briefly summarize the status of #1400 (which this PR supports)? What testing has been done for this new system? What do I need to do to test this myself? What is the status of the merges of the branches? Has Kokkos accepted the PR for your branches? That needs to be done before we can merge this Trilinos PR branch.

trilinos-autotester · 2017-11-29T18:25:10Z

Status Flag 'Pre-Test Inspection' - - This Pull Request Requires Inspection... The code must be inspected by a member of the Team before Testing/Merging
THE LAST COMMIT TO THIS PULL REQUEST HAS NOT BEEN REVIEWED YET!

crtrott · 2017-12-15T00:42:14Z

I don't think that this should go in at all. I believe this comes in as part of the Kokkos-promotion.

…4cd816 From repository at git@github.com:kokkos/kokkos.git At commit: commit 564ec79e3409f9bdd6d1e9a6cadad047764cd816 Merge: 7a06fc8 9614f72 Author: Dan Ibanez <ibaned@users.noreply.github.com> Date: Tue Nov 6 14:27:47 2018 -0700 Merge pull request trilinos#1884 from kokkos/master-backmerge Update master-history

Use new kokkos functionality

afcd4f0

cmake/ProjectCompilerPostConfig.cmake now invokes the kokkos build system to get additional compiler information that is useful for advanced architectures. This enables the useful information that Kokkos calculates can be more widely available.

bartlettroscoe added the stage: in progress Work on the issue has started label Oct 20, 2017

mhoemmen requested changes Oct 22, 2017

View reviewed changes

kruger changed the base branch from master to develop October 22, 2017 20:55

Changes based on MH code review

620c090

bartlettroscoe mentioned this pull request Nov 21, 2017

Get CMake build system to take advantage of kokkos/Makefile.kokkos in setting compilers, compiler options etc. #1400

Closed

Update based on namechange in kokkos.

b6773c0

kruger and others added 2 commits November 29, 2017 12:00

More name change fixes.

e85c264

More name changes.

6e8b7f5

crtrott merged commit 6e8b7f5 into trilinos:develop Dec 15, 2017

crtrott removed the stage: in progress Work on the issue has started label Dec 15, 2017

trilinos-autotester mentioned this pull request Mar 5, 2021

Tpetra MultiVector and BlockMultiVector refactor to remove UVM requirement #8821

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use new kokkos functionality #1884

Use new kokkos functionality #1884

kruger commented Oct 20, 2017

mhoemmen commented Oct 22, 2017

tjfulle commented Oct 22, 2017

mhoemmen left a comment

mhoemmen Oct 22, 2017

mhoemmen Oct 22, 2017

kruger Oct 22, 2017

mhoemmen Oct 22, 2017

mhoemmen Oct 22, 2017

kruger Oct 22, 2017

mhoemmen Oct 22, 2017

bartlettroscoe Nov 20, 2017

mhoemmen Oct 22, 2017

kruger Oct 22, 2017

mhoemmen Oct 22, 2017

kruger Oct 22, 2017

mhoemmen Oct 22, 2017

kruger Oct 22, 2017

trilinos-autotester commented Nov 20, 2017

bartlettroscoe commented Nov 20, 2017

trilinos-autotester commented Nov 29, 2017

crtrott commented Dec 15, 2017

Use new kokkos functionality #1884

Use new kokkos functionality #1884

Conversation

kruger commented Oct 20, 2017

mhoemmen commented Oct 22, 2017

tjfulle commented Oct 22, 2017

mhoemmen left a 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

trilinos-autotester commented Nov 20, 2017

bartlettroscoe commented Nov 20, 2017

trilinos-autotester commented Nov 29, 2017

crtrott commented Dec 15, 2017