search

MPAS-Dev / MPAS-Model

Repository for MPAS models and shared framework releases.
235 stars 312 forks source link

Merge the halo adjoint exchange code for real_2d fields #1138

Closed jim-p-w closed 4 months ago

jim-p-w commented 7 months ago

This PR merges the mpas_dmpar_exch_halo_adj_field2d_real() function from the JCSDA-internal/MPAS-Model repository. Additionally, add unit tests to verify the routine.

The routine handles the adjoint of halo exchange communication of an input field across all processors. It accumulates the values of owned point with the values of halos.

The test verifies that owned cells which should not have their values changed by the adjoint function do remain unchanged. The test also verifies that cells which should have their values updated by the adjoint function are updated.

mgduda commented 7 months ago

Can you update the title to state the purpose of this PR? For examples, you can take a look at some of the PRs that we've recently closed.

jim-p-w commented 7 months ago

I've left an initial set of comments on the changes. More generally, should we add the new mpas_dmpar_exch_halo_adj_field2d_real routine in mpas_dmpar.F as one commit, and then add the unit tests in a second commit?

I'm ambivalent about 1 vs two commits. Is it SOP to add tests in a separate commit? I will update the commits after I've made other suggested changes.

When I update the commits, I propose the comment for merging the product code will be:

Merge mpas_dmpar_exch_halo_adj_field2d_real() from the
JCSDA-internal/MPAS-Model repository.

Note the number of halo layers impacts the number of cells which will be updated by this routine:
The first halo layer will update the owned 'edge' cells, where 'edge' cells are adjacent to qhost cells.
The second halo layer will update owned cells which are adjacent to the 'edge' cells.
The third halo layer will update owned cells which are adjacent to the cells updated by the seconds halo layer, etc.

(The note is also in the header comments for the routine.)

byoung-joo commented 6 months ago

The test code looks good! I left a few comments.

For the documentation, I'd like to leave it to @mgduda .

mgduda commented 5 months ago

@jim-p-w Can you take a look through the newest three commits that I've pushed to see whether they seem reasonable to you (and also to check that I haven't made any errors)?

jim-p-w commented 5 months ago

@jim-p-w Can you take a look through the newest three commits that I've pushed to see whether they seem reasonable to you (and also to check that I haven't made any errors)?

@mgduda Those changes look good to me.

mgduda commented 5 months ago

@jim-p-w Can you update the PR description with a paragraph or two explaining what this PR achieves?