Fix bugs in OpenACC directives and revise machine files #6116
Conversation
|
|
I'm seeing changes here that will have impact for other cases. We may need to find a way to make changes that will only affect MPAS. If MPAS needs a flag, then maybe should only be applied to those sources. If you need |
@ndkeen , Thanks for your instructions. I'm going to revert some changes I made. It seems like the best way to add |
There was a problem hiding this comment.
I also think that we may need to restrict the scope of impact from this changes within MPAS. Please see my comments in my review. I prefer to have a GPU bind suggested in this PR instead of "none". But it may need to coordinate with other developers who may impact to the GPU bind change.
| @@ -107,6 +107,7 @@ list(APPEND MPAS_ADD_ACC_FLAGS | |||
| ${CMAKE_BINARY_DIR}/core_ocean/mode_forward/mpas_ocn_time_integration_rk4.f90 | |||
| ${CMAKE_BINARY_DIR}/core_ocean/mode_forward/mpas_ocn_time_integration_si.f90 | |||
| ${CMAKE_BINARY_DIR}/core_ocean/mode_forward/mpas_ocn_time_integration_split.f90 | |||
| ${CMAKE_BINARY_DIR}/core_ocean/mode_forward/mpas_ocn_time_integration_split_ab2.f90 | |||
There was a problem hiding this comment.
I am not sure how CIME use Depends files, but in my understanding, I think the settings the changes made here needs to be placed in "Depends.pm-gpu.nvidiagpu.cmake" file, not this file, because I assume that nvidiagpu is the compiler for OpenACC compilation on PM-GPU.
| @@ -2,9 +2,13 @@ list(APPEND REDUCE_OPT_LIST | |||
| homme/src/share/derivative_mod_base.F90 | |||
| ) | |||
|
|
|||
| # add accelerator/gpu flags for MPAS files | |||
There was a problem hiding this comment.
According to @ndkeen's comment, this setting might be placed in the Depends file, not in this cmake file, in order to limit the scope of impact to specified MPAS files.
| string(APPEND CPPDEFS " -DFORTRANUNDERSCORE -DNO_R16 -DCPRNVIDIA") | ||
| string(APPEND CPPDEFS_DEBUG " -DYAKL_DEBUG") | ||
| if (compile_threaded) | ||
| string(APPEND CMAKE_C_FLAGS " -mp") | ||
| endif() | ||
| string(APPEND CMAKE_C_FLAGS_RELEASE " -O2") | ||
| string(APPEND CMAKE_C_FLAGS_DEBUG " -g") | ||
| string(APPEND CMAKE_Fortran_FLAGS " -i4 -Mstack_arrays -Mextend -byteswapio -Mflushz -Kieee -DHAVE_IEEE_ARITHMETIC -Mallocatable=03 -DNO_R16 -traceback") | ||
| string(APPEND CMAKE_Fortran_FLAGS " -i4 -Mstack_arrays -Mextend -byteswapio -Mflushz -Kieee -DHAVE_IEEE_ARITHMETIC -Mallocatable=03 -DNO_R16 -traceback -acc") |
There was a problem hiding this comment.
According to @ndkeen's comment, this setting might be placed in the Depends file, not in this cmake file, in order to limit the scope of impact to specified MPAS files.
Change an openacc directive in mpas_ocn_diagnostics.F to avoid getting stuck on pm-gpu.
amametjanov
force-pushed
the
hkang/ocean/pm-gpu-fix
branch
from
b88b0d2 to
95d64d2
Compare
|
Rebased the branch onto latest master and checked that this PR still fixes the hang on PM-GPU: https://my.cdash.org/viewTest.php?buildid=2460796 . |
amametjanov
requested review from
grnydawn
and removed request for
amametjanov
|
@amametjanov , thanks for rebasing this PR! @grnydawn , some of changes I made in this PR were duplicates of #6103 (merged) except for |
Fix bugs in OpenACC directives: - Change an openacc directive in mpas_ocn_diagnostics.F to avoid getting stuck on pm-gpu. Fixes #6127 [BFB]
There was a problem hiding this comment.
@amametjanov, the changes that Hyun made in "mpas_ocn_diagnostics.F." look good to me.
However, my test case (compset: GMPAS-IAF, res: T62_EC30to60E2r2) on Hyun's code branch (hkang/ocean/pm-gpu-fix) fails during building SPIO with the following error message:
"/global/u2/y/youngsun/repos/github/E3SM/externals/scorpio/src/clib/pioc.c", line 1282: error: too many arguments in invocation of macro "adios2_init"
ios->adiosH = adios2_init(ios->adios_comm, adios2_debug_mode_on);
Since this error is not related to the changes in the MPAS code, I believe this PR is ready to be merged.
Fix bugs in OpenACC directives:
getting stuck on pm-gpu.
Fixes #6127
[BFB]