+Add explicit interfaces for MOM_domain_infra #1306
Conversation
Added explicit interfaces for the remaining routines in MOM_domain_infra, including global_field, broadcast_domain, and compute_block_extent, and created a new stand-alone routine MOM_define_layout inside of MOM_domains.F90 to replace the mpp routine that it had previously wrapped. Also added comments explicitly documenting some of the integer parameters that are used to signal staggering or communication patterns. All answers are bitwise identical, but some routines have been recreated within the MOM6 code, in the localized case of MOM_define_layout with a slightly different interface.
Codecov Report
@@ Coverage Diff @@
## dev/gfdl #1306 +/- ##
=========================================
Coverage 45.93% 45.93%
=========================================
Files 234 234
Lines 72079 72097 +18
=========================================
+ Hits 33108 33120 +12
- Misses 38971 38977 +6
Continue to review full report at Codecov.
|
|
This PR passed pipeline testing at https://gitlab.gfdl.noaa.gov/ogrp/MOM6/-/pipelines/11953. |
src/framework/MOM_domain_infra.F90
Outdated
| use mpp_domains_mod, only : mpp_define_io_domain, mpp_define_domains, mpp_deallocate_domain | ||
| use mpp_domains_mod, only : mpp_get_domain_components, mpp_get_domain_extents | ||
| use mpp_domains_mod, only : mpp_get_compute_domain, mpp_get_data_domain, mpp_get_global_domain | ||
| use mpp_domains_mod, only : mpp_get_boundary, mpp_update_domains, global_field_sum => mpp_global_sum |
There was a problem hiding this comment.
From our discussions, it seems that global_field_sum is being left as a pass-through because it is unused in MOM source, but still could be used by existing drivers or other config_src code.
I'd recommend breaking this out into a separate statement, with a comment explaining why this function is unwrapped, explaining that it is informally part of the MOM API but is unused and undocumented.
There was a problem hiding this comment.
I don't know that this code is often examined by other people, and while these are in module use statements for the nuopc and mct couplers, they are not actually used there. Rather than putting in some comments here that will just be ignored (or perhaps as well as), I would like to either (1) put in an issue, or (2) actually remove the unused module use statements from the other couplers, now that we have a way to compile them for testing purposes.
There was a problem hiding this comment.
I still think it's a good idea to document exceptions to the rules, even if it is only referenced by a very small group (perhaps even just ourselves).
In order to avoid creating work for you, I have submitted a PR to your branch which makes this change.
src/framework/MOM_domain_infra.F90
Outdated
| use mpp_domains_mod, only : mpp_get_compute_domain, mpp_get_global_domain | ||
| use mpp_domains_mod, only : mpp_get_domain_extents, mpp_deallocate_domain | ||
| use mpp_domains_mod, only : mpp_update_domains, global_field_sum => mpp_global_sum | ||
| use mpp_domains_mod, only : domain2D, domain1D, group_pass_type => mpp_group_update_type |
There was a problem hiding this comment.
Since group_pass_type is used in the code, does this need to be documented somehow?
There was a problem hiding this comment.
Because this is an opaque type whose contents are never referenced anywhere that is visible in the MOM6 code, I don't think that we need to document its contents (even if we could figure out how to do this).
There was a problem hiding this comment.
See response above, PR addresses this change.
This patch explicitly identifies `global_field_sum` and `group_pass_type` as FMS pass-throughs. These remain undocumented and cannot be wrapped, but the exceptional reasons for retaining them justifies their existence as pass-throughs. Since `global_field_sum` is unused in MOM6/src, it does not require an explicit definition or wrapper. Legacy drivers may still require this definition, however, so we retain it here. The contents of `group_pass_type` are never accessed, and it is only used to facilitate internal mpp operations, so it does not need to formally be part of the MOM6 API and can be left undocumented. Since these two functions are exceptions to the rule that all active operations should be wrapped and documented, it makes sense to explicitly identify them as such.
MOM_domain_infra: Document FMS passthroughs
Added explicit interfaces for the remaining routines in MOM_domain_infra,
including global_field, broadcast_domain, and compute_block_extent, and created
a new stand-alone routine MOM_define_layout inside of MOM_domains.F90 to replace
the mpp routine that it had previously wrapped. Also added comments explicitly
documenting some of the integer parameters that are used to signal staggering or
communication patterns. All answers are bitwise identical, but some routines
have been recreated within the MOM6 code, in the localized case of
MOM_define_layout with a slightly different interface.