Skip to content

Replace diagnosticsPool with module-level variables#611

Closed
mattdturner wants to merge 5 commits intoMPAS-Dev:ocean/developfrom
mattdturner:ocean/diag_solve_optimization
Closed

Replace diagnosticsPool with module-level variables#611
mattdturner wants to merge 5 commits intoMPAS-Dev:ocean/developfrom
mattdturner:ocean/diag_solve_optimization

Conversation

@mattdturner
Copy link
Collaborator

This PR creates a new file mpas_ocn_diagnostics_variables.F that houses all variables in the diagnosticsPool. During init, there is a call to mpas_pool_get_array for each of the variables in the diagnosticsPool, and then other files use those variables via a use ocn_diagnostics_variables statement. This change makes it easier for these arrays to be copied to the GPUs. Note, though, that there is no code in this PR to copy arrays to the GPU.

This PR also changes the diagnostics code to use the ocn_mesh module instead of the meshPool, based on #496.

Merge the changes in PR#457 for mpas_ocn_diagnostics.F that remove
scratch allocates and the calls to mpas_pool_get_config.
Replace the calls to mpas_pool_get_dimension on meshPool
with the use of the ocn_mesh module
Remove all instances (where possible) of meshPool from the diagnostic
routines, their argument list, and the argument lists of subroutines
called by diagnostic routines.
Replace all of the instances of diagnosticsPool with module-level variables.  diagnosticsPool
is only during init now.
Update the edgeSignOnCell and edgeSignOnVertex allocatable arrays in
ocn_init_routines_setup_sign_and_index_fields instead of pointers.  Also change ocn_meshUpdateFields
to update the pointers to match thew allocatables
@mattdturner
Copy link
Collaborator Author

This passes the nightly regression suite on Cori w/ Intel compiler. The results are bfb for performance test (where I did most of my testing), and should be bfb for the entire suite.

Copy link
Contributor

@philipwjones philipwjones left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ran a quick test on Summit/PGI with same results. And by partial inspection (didn't look at all 59 files).

/>
<var name="RediKappaData" type="real" dimensions="nCells" units="m^2 s^{-1}"
description="Redi isopycnal mixing Kappa value. This is the data field specified at init."
packages="gm"
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mark-petersen This was necessary to get a few of the tests in the regression suite to pass. I can revert this change if necessary.

Comment on lines +632 to +640
call ocn_init_routines_setup_sign_and_index_fields()
! Update edgeSignOnCell and edgeSignOnVertex pionters in ocn_mesh. The ocn_mesh allocatables
! are updated in ocn_init_routines_setup_sign_and_index_fields. This updates the pointers to
! Registry to match the allocatable arrays
call mpas_pool_get_array(meshPool, 'edgeSignOnCell', edgeSignOnCellTmp)
call mpas_pool_get_array(meshPool, 'edgeSignOnVertex', edgeSignOnVertexTmp)
edgeSignOnCellTmp = int(edgeSignOnCell)
edgeSignOnVertexTmp = int(edgeSignOnVertex)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@philipwjones Can you check this to make sure there isn't an issue with the way I'm updating the Registry pointers for edgeSignOnCell and edgeSignOnVertex?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, this looks fine.

Comment on lines +1047 to +1051
! Allocatable array was updated in ocn_init_routines_setup_sign_and_index_fields. Now the
! pointer needs to be udpated.
do n = 1, nCellsAll+1
do k = 1, maxEdges
edgeSignOnCell(k, n) = real(edgeSignOnCellTmp(k, n), RKIND)
edgeSignOnCellTmp(k, n) = int(edgeSignOnCell(k, n))
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@philipwjones I don't know if this is necessary anymore, since I updated the pointer in mpas_ocn_init_routines.F. Eventually, every routine should be using the arrays instead of pointers, correct?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Might be good to keep the pointers updated in case someone still accesses via the meshPool. The current mesh module is back-compatible with pools just so we could do incremental development, but we will eventually get rid of all the pointers here (and Registry entries) in favor of just allocatable arrays in this module.

nEdges = nCellsArray( size(nCellsArray) )
nCells = nCellsAll
nVertices = nVerticesAll
nEdges = nCellsAll
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mark-petersen Should line 491 (previously 542) be nEdges = nEdgesAll instead of nEdges = nCellsAll? I based the current line on what was previously there, but it seems odd that we are setting nEdges to nCellsAll...

mark-petersen added a commit that referenced this pull request Nov 10, 2020
…to ocean/develop

New module for ocean diagnostics variables #745

This PR add a new module to house the variables contained in
diagnosticsPool in the ocean model. This also implements the use of the
new module in the mpas_ocn_diagnostics.F file.

This is the first of a handful of PRs that will replace #611 by breaking
it up into smaller PRs.
@mattdturner
Copy link
Collaborator Author

This was replaced by PR #745 , #783 , and an upcoming PR. As such, I am closing this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants