Hypre, outerloop & CUDA changes #2397

bendudson · 2021-08-10T21:26:48Z

Ready for review & merge

Quite a large PR, adding support for GPUs in BOUT++. The two main parts are

A way to write physics models using "outerloop" operators, which merges loops together into larger kernels. This requires new datatypes FieldAccessor and CoordinatesAccessor for efficient access to field and coordinates data respectively.
A 3D phi solver using Hypre. The latest versions of Hypre can run at least in part on GPUs, so this solve can be on the GPU

Includes manual and tests.

…ject/BOUT-dev into next-hypre-outerloop-cuda-merged

Duplicate versions of various field files, VIM temporary files committed to the repo.

Small changes to spacing adding noise, backup files not needed

Otherwise Coordinates is an incomplete type

Whitespace changes and some comments

dz has now changed to being a Coordinates::FieldMetric type, 2D or 3D depending on compile-time option.

In FieldAccessor was only set if the field had parallel slices In Field2DAccessor never set

Never initialised

Needs a field to construct a valid object, so don't allow default construction.

Coordinates fields can now be 2D or 3D, depending on compile-time switch. f2d prefix would just be confusing if it could then be Field3D.

These definitions are repeated in several places, so just move to one location.

Coordinate fields can now be 2D or 3D, depending on compile-time flag BOUT_USE_METRIC_3D. This puts the logic for indexing these fields in one place.

The Field2DAccessor code was almost the same as FieldAccessor. Rather than having two implementations, use the same implementation and specialise on Field2D types.

`f_data` was a duplicate of `data`, and other `f_` prefixes aren't needed.

Moved to field_accessor.hxx rather than single_index_ops.hxx These changes make the API more similar to the Fields.

Uses the build flags, hopefully will work for all cases on the GPU

Just indexes data array, so `fa[i]` is equivalent to `fa.data[i]`

Compiler errors, include Mesh header

Bugs in `b0xGrad_dot_Grad`, `D2DY2` etc. fixed Renamed, removing `_g` postfix. Functions can take either an `int` or an `Ind3D`. The `int` argument is assumed to be a 3D index in all cases.

Compiles with 2D metrics and no RAJA (CPU only).

Document FieldAccessor and CoordinateFieldAccessor

A simple wrapper around `BoutReal*`, which has an indexing (subscript) operator for `int` and `Ind3D` types (const and non-const).

Code can be the same, but the surrounding loop is currently different.

Now compiles again without RAJA & CUDA. Previously also had braces in the wrong location.

Can be used by both LaplaceXY2 and LaplaceXY2Hypre, so put into a common header file

f2dinit member variable no longer needed for initialisation

The PetscMatrix API (constructor args) had changed, but this LaplaceXY2 implementation had not been updated. Now compiles again, though not tested that it runs or produces correct result.

Outerloop suggestions - resolve merge conflicts

Previously nvcc would fail claiming that symbols were defined multiple times. Now needs these symbols to be defined or linker errors result.

HYPRE_SOLVER_TYPE can't be directly formatted with nvcc compiler, but needs to be first converted to a string.

More cori nvcc fixes

Outerloop suggestions

bendudson · 2022-05-19T20:57:21Z

@ZedThree @johnomotani @jonesholger @dschwoerer I think this is now finally ready to be merged!

ZedThree · 2022-05-20T16:52:20Z

tests/unit/sys/test_raja.cxx

+#if 0 // disable temporarily until reconcile iteration space for parallel_forall  under
+      // nvcc


Any update on this?

ZedThree · 2022-05-20T16:53:28Z

tests/unit/sys/test_timer.cxx

+// Don't currently know why this test fails, and also causes segfault when unwinding the
+// tests
+#if 1


Is this #2406 ?

ZedThree · 2022-05-20T16:56:43Z

I've got most of the way through this, will finish it when I get back from holiday! But I have a feeling it's "good enough". There's a few bits and pieces that could be tidied up -- we should probably make issues for them and try and clean them up later perhaps.

malloc/free in the Hypre interface
malloc in tests
a few #if 0/1 scattered in places that should be removed or at least investigated/issues made

bendudson · 2022-05-20T17:17:31Z

Thanks @ZedThree, have a good holiday!

ZedThree

LGTM. Obviously this is a really big PR, but I think the general shape is good.

There's a few things that I'd really like to fix at some point, so noting here perhaps for future issues:

Use of std::shared_ptr where it might not be needed
Use of malloc/new
Timer tests have some #if 1/0 bits (might be related to TimerTest.ListAllInfo only works once #2406 ?)

include/bout/invert/laplacexy2.hxx

- Added to CMakeLists; previously wasn't compiled with CMake builds - Removed shared_ptr around PetscMatrix - Moved initialisation of indexConverter and matrix to initializer list

Laplacexy2 modifications

Only makes sense with 2D metrics, fails to compile if 3D metrics are enabled.

bendudson · 2022-06-23T15:45:07Z

ok @ZedThree I think this is now finally ready to go in

ZedThree · 2022-06-23T15:49:33Z

clang-tidy-review failed because: "CMake 3.17 or higher is required. You are running version 3.16.3"

We can update that elsewhere.

bendudson added work in progress Not ready for merging feature performance labels Aug 10, 2021

bendudson added 27 commits August 11, 2021 05:38

Merge branch 'next-hypre-outerloop-cuda-merged' of github.com:boutpro…

7bc20aa

…ject/BOUT-dev into next-hypre-outerloop-cuda-merged

Remove unnecessary backup/temp files

a624b3b

Duplicate versions of various field files, VIM temporary files committed to the repo.

Small tidying, deleting duplicates

d47046a

Small changes to spacing adding noise, backup files not needed

Include coordinates.hxx in FieldAccessor headers

f14f734

Otherwise Coordinates is an incomplete type

Clang formatting FieldAccessor files

1fd15d4

Whitespace changes and some comments

Convert f2d_dz from BoutReal to pointer

5fd4c76

dz has now changed to being a Coordinates::FieldMetric type, 2D or 3D depending on compile-time option.

Fix FieldAccessor::f_ddt initialisation

dffbaaf

In FieldAccessor was only set if the field had parallel slices In Field2DAccessor never set

Remove ind_{2,3}D_index members from FieldAccessor

9a5557b

Never initialised

Delete FieldAccessor default constructor

65aa3a9

Needs a field to construct a valid object, so don't allow default construction.

Remove f2d_ prefix from FieldAccessor coordinates

09d12b1

Coordinates fields can now be 2D or 3D, depending on compile-time switch. f2d prefix would just be confusing if it could then be Field3D.

Move BOUT_HOST_DEVICE etc into build_config.hxx

65c9b01

These definitions are repeated in several places, so just move to one location.

Add CoordinateFieldAccessor type

27cd64a

Coordinate fields can now be 2D or 3D, depending on compile-time flag BOUT_USE_METRIC_3D. This puts the logic for indexing these fields in one place.

Make Field2DAccessor a specialisation

948a4ec

The Field2DAccessor code was almost the same as FieldAccessor. Rather than having two implementations, use the same implementation and specialise on Field2D types.

Rename FieldAccessor members

1104663

`f_data` was a duplicate of `data`, and other `f_` prefixes aren't needed.

Rename DDT(FieldAccessor) to ddt(FieldAccessor)

233ced9

Moved to field_accessor.hxx rather than single_index_ops.hxx These changes make the API more similar to the Fields.

Annotate SQ() with BOUT_HOST_DEVICE

2a0ade9

Uses the build flags, hopefully will work for all cases on the GPU

Add operator[] to FieldAccessor

5d1ac09

Just indexes data array, so `fa[i]` is equivalent to `fa.data[i]`

Small fixes to CoordinateFieldAccessor

c988ac8

Compiler errors, include Mesh header

Many changes, fixes to single_index_ops.hxx

8e59096

Bugs in `b0xGrad_dot_Grad`, `D2DY2` etc. fixed Renamed, removing `_g` postfix. Functions can take either an `int` or an `Ind3D`. The `int` argument is assumed to be a 3D index in all cases.

Clang format haswgawa-wakatani-3d

a31ea4c

Compiles with 2D metrics and no RAJA (CPU only).

Adding some docstrings

4cb3ac2

Document FieldAccessor and CoordinateFieldAccessor

Add BoutRealArray to FieldAccessor

20d480a

A simple wrapper around `BoutReal*`, which has an indexing (subscript) operator for `int` and `Ind3D` types (const and non-const).

Hasegawa-Wakatani-3D merge RAJA and CPU code

1f4b7c8

Code can be the same, but the surrounding loop is currently different.

Fixes and reformatting elm-pb code

6c36409

Now compiles again without RAJA & CUDA. Previously also had braces in the wrong location.

Move squareStencil<> to operatorstencil.hxx

1e2729b

Can be used by both LaplaceXY2 and LaplaceXY2Hypre, so put into a common header file

Remove unused code in LaplaceXY2Hypre

e95b6a7

f2dinit member variable no longer needed for initialisation

Fix LaplaceXY2 implementation

c6f9106

The PetscMatrix API (constructor args) had changed, but this LaplaceXY2 implementation had not been updated. Now compiles again, though not tested that it runs or produces correct result.

bendudson and others added 10 commits May 16, 2022 16:58

Merge pull request #2525 from boutproject/outerloop-suggestions-bd

c6c4907

Outerloop suggestions - resolve merge conflicts

Apply clang-format changes

d1f37d6

Update fmt to latest version

f8c0bff

nvcc now seems to need the factory flags

7cb4ff5

Previously nvcc would fail claiming that symbols were defined multiple times. Now needs these symbols to be defined or linker errors result.

nvcc needs toString(HYPRE_SOLVER_TYPE)

c7ca929

HYPRE_SOLVER_TYPE can't be directly formatted with nvcc compiler, but needs to be first converted to a string.

Apply clang-format changes

ebc234e

Merge pull request #2547 from boutproject/more-cori-nvcc-fixes

fd3c3dc

More cori nvcc fixes

Merge pull request #2495 from boutproject/outerloop-suggestions

08f25c6

Outerloop suggestions

Merge branch 'next' into next-hypre-outerloop-cuda-merged

33299d8

Apply clang-format changes

1d5ac44

ZedThree reviewed May 20, 2022

View reviewed changes

ZedThree previously approved these changes Jun 13, 2022

View reviewed changes

include/bout/invert/laplacexy2.hxx Outdated Show resolved Hide resolved

bendudson and others added 3 commits June 15, 2022 10:07

Laplacexy2 modifications

cdc5df0

- Added to CMakeLists; previously wasn't compiled with CMake builds - Removed shared_ptr around PetscMatrix - Moved initialisation of indexConverter and matrix to initializer list

Apply clang-format changes

9f39408

Merge pull request #2551 from boutproject/next-merged-laplacexy2

56bfb3a

Laplacexy2 modifications

ZedThree dismissed their stale review via 56bfb3a June 22, 2022 15:14

Apply clang-format changes

44e39d8

ZedThree previously approved these changes Jun 22, 2022

View reviewed changes

Don't compile LaplaceXY2 with 3D metrics

a9dad13

Only makes sense with 2D metrics, fails to compile if 3D metrics are enabled.

bendudson dismissed ZedThree’s stale review via a9dad13 June 22, 2022 17:55

Apply clang-format changes

606183c

ZedThree approved these changes Jun 23, 2022

View reviewed changes

ZedThree merged commit d34cd44 into next Jun 23, 2022

ZedThree deleted the next-hypre-outerloop-cuda-merged branch June 23, 2022 15:50

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Hypre, outerloop & CUDA changes #2397

Hypre, outerloop & CUDA changes #2397

bendudson commented Aug 10, 2021 •

edited

bendudson commented May 19, 2022

ZedThree May 20, 2022

ZedThree May 20, 2022

ZedThree commented May 20, 2022

bendudson commented May 20, 2022

ZedThree left a comment

bendudson commented Jun 23, 2022

ZedThree commented Jun 23, 2022

		#if 0 // disable temporarily until reconcile iteration space for parallel_forall under
		// nvcc

Hypre, outerloop & CUDA changes #2397

Hypre, outerloop & CUDA changes #2397

Conversation

bendudson commented Aug 10, 2021 • edited

bendudson commented May 19, 2022

ZedThree May 20, 2022

Choose a reason for hiding this comment

ZedThree May 20, 2022

Choose a reason for hiding this comment

ZedThree commented May 20, 2022

bendudson commented May 20, 2022

ZedThree left a comment

Choose a reason for hiding this comment

bendudson commented Jun 23, 2022

ZedThree commented Jun 23, 2022

bendudson commented Aug 10, 2021 •

edited