Skip to content
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

Rename MOM_ensemble_manager to MOM_ensemble_manager_infra #1303

Merged

Conversation

MJHarrison-GFDL
Copy link
Contributor

Pass through interfaces to FMS with documentation.

  - Pass throught interfaces to FMS with documentation.
@codecov-io
Copy link

codecov-io commented Jan 27, 2021

Codecov Report

Merging #1303 (932c29d) into dev/gfdl (790ffbe) will decrease coverage by 0.00%.
The diff coverage is 27.77%.

Impacted file tree graph

@@             Coverage Diff              @@
##           dev/gfdl    #1303      +/-   ##
============================================
- Coverage     45.93%   45.92%   -0.01%     
============================================
  Files           233      234       +1     
  Lines         72052    72070      +18     
============================================
+ Hits          33094    33099       +5     
- Misses        38958    38971      +13     
Impacted Files Coverage Δ
src/framework/MOM_ensemble_manager_infra.F90 27.77% <27.77%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 790ffbe...4798234. Read the comment docs.

@Hallberg-NOAA
Copy link
Collaborator

The documentation of the interfaces looks great - thank you for taking this on. However there are two issues with this commit that we might want consider.

First, by renaming the file, there is now an inconsistency between the file and module names. If the file name is MOM_ensemble_manager_infra.F90, the module should be MOM_ensemble_manager_infra, not MOM_ensemble_manager.

The second is a structural issue with this commit. The plan is that all of the infrastructure specific modules (like ensemble_manager_infra.F90 ) will move out of the MOM6/src/framework directory into a directory like MOM6/config_src/infra/FMS1/ . However at that point, there should not be any calls to the config_src/infra code from other parts of MOM6 outside of the framework directory. Instead there should be two files, (say MOM_ensemble_manager.F90 and MOM_ensemble_infra.F90), with the calls going via MOM_ensemble_manager to MOM_ensemble_infra and then into the FMS or (or other) infrastructure. This way hopefully we may be able to buffer any interface changes that we find we need to make to accommodate other infrastructures from requiring changes elsewhere in the MOM6 code (like in the ODA code), or even in routines that we don't know about because other users have not checked them into dev/gfdl or the other main forks.

As an example of what we are thinking, please see the revised MOM_cpu_clock.F90 and MOM_cpu_clock_infra.F90. In this case, it looks like all of the routines in MOM_ensemble_manager.F90 might just be pass-throughs to MOM_ensemble_infra.F90.

@Hallberg-NOAA
Copy link
Collaborator

This PR has passed pipeline testing at https://gitlab.gfdl.noaa.gov/ogrp/MOM6/-/pipelines/11950.

Copy link
Collaborator

@Hallberg-NOAA Hallberg-NOAA left a comment

Choose a reason for hiding this comment

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

This PR does a nice job of documenting the interfaces that MOM6 uses to the ensemble_manager code, it matches the new structure we are moving toward, and it has passed all of our TC and pipeline testing.

@Hallberg-NOAA Hallberg-NOAA merged commit 5a938c8 into mom-ocean:dev/gfdl Jan 27, 2021
@MJHarrison-GFDL MJHarrison-GFDL deleted the ensemble_manager_infra branch December 29, 2021 19:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants