-
Notifications
You must be signed in to change notification settings - Fork 548
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
MPI communicator generalizations #1277
base: develop
Are you sure you want to change the base?
Changes from 16 commits
1ef96e9
51967ea
1e561f8
6a4c276
98b8425
f2810fb
5805c9b
ba8db4c
7805f33
53bbe18
a285a6a
9f34f74
863f97e
ee61569
3cb627c
c9bd906
cf3d25c
18a9e51
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,6 +3,7 @@ set(c_src w3getmem.c) | |
# Core files always built | ||
set(ftn_src | ||
constants.F90 | ||
mpicomm.F90 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's unclear to me if this should only be included if you have the MPI switch and therefore be added in model/src/cmake/switches.json instead. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I did try to include ifdefs to make mpicomm.F90 not affect anything if it was compiled without MPI, but I can try to change this to model/src/cmake/switches.json since that sounds cleaner (I will wait on this fix for further clarity on whether a separate module is needed) |
||
w3adatmd.F90 | ||
w3arrymd.F90 | ||
w3bullmd.F90 | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -282,7 +282,7 @@ | |
}, | ||
{ | ||
"name": "NL2", | ||
"build_files": ["w3snl2md.F90", "mod_xnl4v5.f90", "serv_xnl4v5.f90", "mod_fileio.f90", "mod_constants.f90"], | ||
"build_files": ["w3snl2md.F90", "mod_xnl4v5.f90", "serv_xnl4v5.f90", "mod_fileio.f90", "mod_constants.f90", "mod_mpicomm.f90"], | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I do not see a file mod_mpicomm.f90. Also I think mpicomm.F90 should be adde with the MPI switch not NL2. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I can revert this change, I was initially confused about whether mod files were separately needed |
||
"conflicts": ["OMPG", "OMPH"] | ||
}, | ||
{ | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,67 @@ | ||
!> @file | ||
!> @brief Defines MPI communicator id as constants for global use. | ||
!> | ||
!> @author H. L. Tolman @date 05-Jun-2018 | ||
!> | ||
#include "w3macros.h" | ||
|
||
!> | ||
!> @brief Define some mpi constants for global use | ||
!> | ||
!> @author J. M. Sexton @date 19-Jul-2024 | ||
!> | ||
! | ||
#ifndef ENDIANNESS | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can you help me understand why this is added here? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This file was modelled off of constants, this ifdef is probably not needed. |
||
#define ENDIANNESS "native" | ||
#endif | ||
! | ||
!/ ------------------------------------------------------------------- / | ||
MODULE MPICOMM | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is a fairly lightweight routine, should we add this to an existing file such as constants? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not sure what the ramifications would be here, I'll consult with one of my collaborators who'd originally suggested having it as a separate file. I believe the intent was to make it more portable and obviously defined. Would w3parall.F90 be appropriate? I do see that There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I can move things to another location if that's your preference. When you know, can you point me to where specifically so that I can push an update? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. w3parall is mostly for unstructured grids, where as this is for all grids, so I think constants might be a better choice. Others can weigh in on this thought too. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks for the explanation, and I'm also interested in others thoughts 🙂 I was originally looking at w3parall because the SYNCHRONIZE_GLOBAL_ARRAY has been useful in our coupling tests so far, although I do see now that the header for this file indicates it's "Parallel routines for implicit solver" |
||
!/ | ||
!/ +-----------------------------------+ | ||
!/ | WAVEWATCH III NOAA/NCEP | | ||
!/ | H. L. Tolman | | ||
!/ | FORTRAN 90 | | ||
!/ | Last update : 19-Jul-2024 | | ||
!/ +-----------------------------------+ | ||
!/ | ||
!/ 19-Jul-2024 : Add MPI SubCommunicator variable ( version 7.xx ) | ||
!/ | ||
!/ Copyright 2009-2024 National Weather Service (NWS), | ||
!/ National Oceanic and Atmospheric Administration. All rights | ||
!/ reserved. WAVEWATCH III is a trademark of the NWS. | ||
!/ No unauthorized use without permission. | ||
!/ | ||
! 1. Purpose : | ||
! | ||
! Define some mpi constants for global use | ||
! | ||
! 2. Variables and types : | ||
! | ||
! Name Type Scope Description | ||
! ---------------------------------------------------------------- | ||
! MPI_COMM_WW3 Int Global Value for mpi (sub)communicator for WW3 | ||
! IS_EXTERNAL_COMPONENT Bool Global General logical similar to IS_ESMF_COMPONENT | ||
! ---------------------------------------------------------------- | ||
!/ ------------------------------------------------------------------- / | ||
!/ | ||
! | ||
!#ifdef W3_MPI | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please clean up this commented out code. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. are you referring to the ifdef W3_MPI here? I was unsure if I followed the header convention correctly |
||
! INCLUDE "mpif.h" | ||
!#endif | ||
|
||
INTEGER :: MPI_COMM_WW3=0 !< MPI_COMM_WW3 | ||
! | ||
! Parameters in support of running as ESMF component | ||
! | ||
! --- Flag indicating whether or not the model has been invoked as an | ||
! external Component. This flag is set to true in the external | ||
! module during initialization. | ||
LOGICAL :: IS_EXTERNAL_COMPONENT = .FALSE. !< IS_EXTERNAL_COMPONENT Flag for model invoked via external executable. | ||
! | ||
CONTAINS | ||
|
||
!/ | ||
!/ End of module MPICOMM ------------------------------------------- / | ||
!/ | ||
END MODULE MPICOMM |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -410,6 +410,7 @@ SUBROUTINE W3INIT ( IMOD, IsMulti, FEXT, MDS, MTRACE, ODAT, FLGRD, FLGR2, FLGD, | |
UA, UD, U10, U10D, AS | ||
#ifdef W3_MPI | ||
USE W3ADATMD, ONLY: MPI_COMM_WAVE, MPI_COMM_WCMP | ||
USE MPICOMM | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. For all of these USE MPICOM, if we can add the "only" statement that would be nice. I know it's not uniformly done in WW3 right now, but since this is an addition, it would be good to add it. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'll push a commit to make these all have an only statement |
||
#endif | ||
USE W3IDATMD, ONLY: FLLEV, FLCUR, FLWIND, FLICE, FLTAUA, FLRHOA,& | ||
FLMDN, FLMTH, FLMVS, FLIC1, FLIC2, FLIC3, & | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are these changes intentional?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These changes were from attempting to test with debug flags on, but I should revert them (particularly as that seemed to not be a good way to build with debug)