Updated Field API variants of the clouds dwarf (CPU and GPU) #96

wertysas · 2024-10-01T09:16:25Z

This is an update of the Field API variants of cloudsc that aims to resemble the use of Field API in the IFS more closely than before. This includes the addition of a new global state type for the Field API variants: CLOUDSC_FIELD_STATE_TYPE and three types for aux vars, fluxes and tendencies that resemble similar types in the IFS:

CLOUDSC_AUX_TYPE
CLOUDSC_FLUX_TYPE
CLOUDSC_STATE_TYPE

There has also been updates to the build structure, that makes it possible to build the CPU Field API version without OpenACC or CUDA and to build the GPU Field API version without CUDA support. The following ecbuild_options have been added to support this:

with-field-api enables field-api variants to be built
with-mapped-fields builds field-api variants with field pointers automatically by OpenACC to device (requires OpenACC and CUDA)
with-pinned-fields builds field-api variants with fields allocated in pinned memory (requires OpenACC and CUDA)

FussyDuck · 2024-10-01T09:16:31Z

All committers have signed the CLA.

…i GPU version

… IFS.

reuterbal

Many thanks, this looks like it was a substantial piece of work that has been carried out very diligently and well structured!

I left a few small comments but nothing major.

To make sure this is tested in CI, please add --with-field-api in all four entries in the github workflow here:

dwarf-p-cloudsc/.github/workflows/build.yml

Lines 35 to 38 in 57e95e8

    
           - ''  # Plain build without any options 
        
           - '--with-gpu --with-loki --with-atlas' # Enable Loki, Atlas, and GPU variants 
        
           - '--with-gpu --with-loki --with-atlas --with-mpi' # Enable Loki, Atlas, and GPU variants with MPI 
        
           - '--single-precision --with-gpu --with-loki --with-atlas --with-mpi' # Enable Loki, and GPU variants with MPI in a single-precision build

Further down, you may have to add the GPU-enabled field_api build target also to the ctest_exclude options, e.g., here:

dwarf-p-cloudsc/.github/workflows/build.yml

Line 57 in 57e95e8

    
           ctest_exclude_pattern: '-gpu-|-scc-|-loki-c|-cuda-'  # GPU variants don't work on CPU runners, loki-c variant causes SIGFPE

In addition, you will have to add the new target to the script here: https://github.com/ecmwf-ifs/dwarf-p-cloudsc/blob/develop/.github/scripts/verify-targets.sh

CMakeLists.txt

bundle.yml

src/cloudsc_fortran/CMakeLists.txt

src/cloudsc_gpu/CMakeLists.txt

src/common/module/cloudsc_field_state_mod.F90

reuterbal · 2024-10-15T22:03:51Z

Apologies, final request: Please update also the README section here:

dwarf-p-cloudsc/README.md

Lines 81 to 90 in 57e95e8

    
           - **dwarf-cloudsc-gpu-scc-field**: GPU-enabled and optimized version of 
        
             CLOUDSC that uses the SCC loop layout, and uses [FIELD API](https://github.com/ecmwf-ifs/field_api) (a Fortran library purpose-built for IFS data-structures that facilitates the 
        
             creation and management of field objects in scientific code) to perform device offload  
        
             and copyback. The intent is to demonstrate the explicit use of pinned host memory to speed-up  
        
             data transfers, as provided by the shipped prototype implmentation, and  
        
             investigate the effect of different data storage allocation layouts.  
        
             To enable this variant, a suitable CUDA installation is required and the 
        
             `--with-cuda` flag needs to be passed at the build stage. This variant lets the CUDA runtime  
        
             manage temporary arrays and needs a large `NV_ACC_CUDA_HEAPSIZE`  
        
             (eg. `NV_ACC_CUDA_HEAPSIZE=8GB` for 160K columns.)

awnawab

Many thanks @wertysas for this excellent and comprehensive piece of work! 🙏 Mirroring the F-API usage in the IFS is really great as it will allow us to test F-API related Loki recipes on cloudsc first.

I've left a few comments for you to address, but none of these require major changes.

CMakeLists.txt

bundle.yml

src/common/module/cloudsc_aux_type_mod.F90

src/common/module/cloudsc_flux_type_mod.F90

src/common/module/cloudsc_state_type_mod.F90

src/cloudsc_gpu/cloudsc_driver_gpu_scc_field_mod.F90

bundle.yml

…-api flag is set but --with-cuda is turned off.

wertysas · 2024-10-17T13:36:12Z

src/cloudsc_loki/CMakeLists.txt

+            ${COMMON_MODULE}/cloudsc_field_state_mod.F90
+            ${COMMON_MODULE}/cloudsc_flux_type_mod.F90
+            ${COMMON_MODULE}/cloudsc_aux_type_mod.F90
+            ${COMMON_MODULE}/cloudsc_state_type_mod.F90


This should probably not be added here, since this is the SCC CUF variant that is not based on the field variant of the gpu driver. My thinking is that a new loki_transform will be added through the dwarf-p-cloudsc/je-field-api-offload branch that I am using for development of the Loki Field API offload transformation.

…nned memory option from compile time constant to runtime ENV variable

awnawab · 2024-10-24T07:22:40Z

I think the gnu --with-field and --with-gpu CI builds are failing because gnu has some limited openacc support but is untested for F-API and so it shouldn't be enabled. You can add --cmake="FIELD_API_ENABLE_ACC=OFF" to the build flags for the gnu runners.

…pped-field to without-mapped-fields, added field variants to verify-targets, and updated sync_host methods in scc_field driver

…NIT_MAP_DEVTPR logic to field_state_mod

…nd added CPU field version to README

awnawab · 2024-10-30T10:42:16Z

Many thanks @wertysas for addressing all of my comments! 🙏 The CI fails at the moment because it can't find the scc-field targets. This is because the condition for that is field_api_HAVE_ACC which we explicitly disable. Changing that condition to HAVE_ACC should fix it. You might also want to set the env variable DEV_ALLOC_SIZE=1073741824 (1Gb) for the F-API gpu enabled unit tests.

…. variable to field gpu tests

awnawab

Thanks a lot for sorting out the CI issues, this looks great and good to go 👌

reuterbal

Many thanks for the updates and @awnawab for the diligent review! Just one final request to use a more meaningful option description, otherwise ready to go!

reuterbal · 2024-10-30T13:32:07Z

.github/workflows/build.yml

+          - arch: nvhpc/21.9
+            nvhpc_version: 21.9
+            io_library_flag: ''
+            build_flags: '--single-precision --with-gpu --with-loki --with-cuda --with-field'
+            ctest_exclude_pattern: '-gpu-|-scc-|-loki-c|-cuda'  # GPU variants don't work on CPU runners, loki-c variant causes SIGFPE
+          - arch: nvhpc/21.9
+            nvhpc_version: 21.9
+            io_library_flag: '--with-serialbox'
+            build_flags: '--with-gpu --with-loki --with-cuda --with-field'
+            ctest_exclude_pattern: '-gpu-|-scc-|-loki-c|-cuda'  # GPU variants don't work on CPU runners, loki-c variant causes SIGFPE
+          - arch: nvhpc/21.9
+            nvhpc_version: 21.9
+            io_library_flag: ''
+            build_flags: '--single-precision --with-gpu --with-loki --with-cuda --with-field --without-mapped-fields'
+            ctest_exclude_pattern: '-gpu-|-scc-|-loki-c|-cuda'  # GPU variants don't work on CPU runners, loki-c variant causes SIGFPE
+          - arch: nvhpc/21.9
+            nvhpc_version: 21.9
+            io_library_flag: '--with-serialbox'
+            build_flags: '--with-gpu --with-loki --with-cuda --with-field --without-mapped-fields'
+            ctest_exclude_pattern: '-gpu-|-scc-|-loki-c|-cuda'  # GPU variants don't work on CPU runners, loki-c variant causes SIGFPE


[No action required!] I think we can skip the 21.9 builds. That compiler version is fairly outdated now and the 23.5 tests are much more meaningful. But that can be dealt with in a subsequent CI-cleanup that is on the horizon.

reuterbal · 2024-10-30T13:35:12Z

CMakeLists.txt

    DEFAULT ON )

 ecbuild_find_package( NAME loki )
 ecbuild_find_package( NAME atlas )

+ecbuild_add_option( FEATURE FIELD_API_DISABLE_MAPPED_MEMORY
+  DESCRIPTION "Use ACC mapped memory by default in Field API objects"


That description should probably read something like "Disable the use of ACC mapped memory in Field API objects"

Yes, thank you for spotting that, I missed to update the description when changing this option. I have changed it now.

mlange05

Looks fantastic! Great contribution and enables a lot of great follow-on stuff.

Driver structure and data types look much closer to the full system in this and the structural issue are solved quite elegantly (eg. the packed FIELD_GANG implementation.

GTG from me.

wertysas requested review from mlange05 and reuterbal October 1, 2024 09:16

FIELD state types for cloudsc skeleton

3dc7920

wertysas force-pushed the je-field-api-view-updates branch from 00d866e to 9754c1a Compare October 1, 2024 09:59

wertysas requested a review from awnawab October 1, 2024 13:03

wertysas marked this pull request as draft October 2, 2024 08:01

wertysas added 5 commits October 2, 2024 16:09

Added Field API view functionality to field state module and Field ap…

4f0ab23

…i GPU version

Added aux, flux and state types using field API for storage as in the…

50ba3b5

… IFS.

Fortran CPU version modified to use Field API as in the IFS

8e0591c

Updated bundle and CMake, added with-field api option

32828de

moved aux, state and flux types into their own modules

99f4da3

wertysas force-pushed the je-field-api-view-updates branch from 7eee6fb to 3804590 Compare October 2, 2024 14:16

wertysas added 3 commits October 4, 2024 10:33

CMake and bundle updates

45a31c0

Updated authors list

6796466

Restoring modified comments

91f01f2

wertysas force-pushed the je-field-api-view-updates branch from a62f0ce to 91f01f2 Compare October 4, 2024 08:52

wertysas marked this pull request as ready for review October 4, 2024 16:14

reuterbal requested changes Oct 15, 2024

View reviewed changes

awnawab requested changes Oct 16, 2024

View reviewed changes

wertysas added 3 commits October 17, 2024 11:57

Cmake and bundle updates after PR comments

97eb718

Updated github CI builds

0b52301

Updated Field API version in bundle and disables CUDA if --with-field…

611b385

…-api flag is set but --with-cuda is turned off.

wertysas commented Oct 17, 2024

View reviewed changes

wertysas added 2 commits October 21, 2024 15:14

Updated call signature of GPU field variant and switched field api pi…

77d19d2

…nned memory option from compile time constant to runtime ENV variable

README updated and bug fixes in field gpu driver and CMake

a8cc3a0

wertysas added 2 commits October 24, 2024 14:57

Switched F-API fields to mapped by default and changed option with-ma…

b5a13c8

…pped-field to without-mapped-fields, added field variants to verify-targets, and updated sync_host methods in scc_field driver

added missing ENABLE before mapped feature in bundle and moved back I…

a3bc0eb

…NIT_MAP_DEVTPR logic to field_state_mod

wertysas added 2 commits October 24, 2024 17:02

Updated workflows and tests to handle without-mapped-fields option, a…

d37f273

…nd added CPU field version to README

Passing flag to CMake to prevent F-API from breaking GNU workflows

125741e

replaced gnu gpu tests with cpu tests, added F-API DEV_ALLOC_SIZE env…

e455384

…. variable to field gpu tests

awnawab approved these changes Oct 31, 2024

View reviewed changes

reuterbal requested changes Nov 4, 2024

View reviewed changes

CMake clean

128382c

mlange05 approved these changes Nov 4, 2024

View reviewed changes

reuterbal approved these changes Nov 4, 2024

View reviewed changes

reuterbal merged commit 3d5c82a into develop Nov 4, 2024
32 checks passed

reuterbal deleted the je-field-api-view-updates branch November 4, 2024 21:13

wertysas mentioned this pull request Nov 21, 2024

New Field API Loki variant with state types in driver #100

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Updated Field API variants of the clouds dwarf (CPU and GPU) #96

Updated Field API variants of the clouds dwarf (CPU and GPU) #96

wertysas commented Oct 1, 2024 •

edited

Loading

FussyDuck commented Oct 1, 2024 •

edited

Loading

reuterbal left a comment

reuterbal commented Oct 15, 2024

awnawab left a comment

wertysas Oct 17, 2024

awnawab commented Oct 24, 2024 •

edited

Loading

awnawab commented Oct 30, 2024

awnawab left a comment

reuterbal left a comment

reuterbal Oct 30, 2024

reuterbal Oct 30, 2024

wertysas Nov 4, 2024

mlange05 left a comment

	- '' # Plain build without any options
	- '--with-gpu --with-loki --with-atlas' # Enable Loki, Atlas, and GPU variants
	- '--with-gpu --with-loki --with-atlas --with-mpi' # Enable Loki, Atlas, and GPU variants with MPI
	- '--single-precision --with-gpu --with-loki --with-atlas --with-mpi' # Enable Loki, and GPU variants with MPI in a single-precision build

Updated Field API variants of the clouds dwarf (CPU and GPU) #96

Updated Field API variants of the clouds dwarf (CPU and GPU) #96

Conversation

wertysas commented Oct 1, 2024 • edited Loading

FussyDuck commented Oct 1, 2024 • edited Loading

reuterbal left a comment

Choose a reason for hiding this comment

reuterbal commented Oct 15, 2024

awnawab left a comment

Choose a reason for hiding this comment

wertysas Oct 17, 2024

Choose a reason for hiding this comment

awnawab commented Oct 24, 2024 • edited Loading

awnawab commented Oct 30, 2024

awnawab left a comment

Choose a reason for hiding this comment

reuterbal left a comment

Choose a reason for hiding this comment

reuterbal Oct 30, 2024

Choose a reason for hiding this comment

reuterbal Oct 30, 2024

Choose a reason for hiding this comment

wertysas Nov 4, 2024

Choose a reason for hiding this comment

mlange05 left a comment

Choose a reason for hiding this comment

wertysas commented Oct 1, 2024 •

edited

Loading

FussyDuck commented Oct 1, 2024 •

edited

Loading

awnawab commented Oct 24, 2024 •

edited

Loading