Skip to content

FMS1 Framework integration#1342

Merged
marshallward merged 10 commits into
mainfrom
gfdl-fms2
Mar 8, 2021
Merged

FMS1 Framework integration#1342
marshallward merged 10 commits into
mainfrom
gfdl-fms2

Conversation

@marshallward
Copy link
Copy Markdown
Collaborator

@marshallward marshallward commented Feb 26, 2021

This pull request introduces the new config_src layout, which includes dedicated directories for the framework infrastructure, couplers, drivers, and memory preprocessing.

Reviewers requested:

Existing build scripts with explicit paths to config_src are unlikely to work after this merge. This includes calls to mkmf or any compiler flags containing directories. Please carefully confirm that you are still able to build and run your models with this PR.

The FMS1 implementation is now moved into config_src/infra/FMS1, and src should no longer contain any explicit calls to FMS.

Couplers and drivers have been renamed and moved into the drivers directory. Many are now denoted with the _cap suffix, e.g.

  • coupled_driver -> FMS_cap
  • mct_driver -> mct_cap
  • nuopc_driver -> nuopc_cap

MOM_memory.h headers are now moved into the config_src/memory directory.

The documentation, build system, and verification tests paths have also been updated to reflect this change.

Also, the target build in the testing will now use its own configure.ac rather than the current repository. In particular, the current regression tests can now use the legacy layout, and future directory tree changes will not break the regression tests.

Commits:

Contributors:

marshallward and others added 10 commits February 24, 2021 15:43
  Created the new config_src directory tree structure, as agreed upon at
https://github.com/NOAA-GFDL/MOM6/discussions/1286, to eventually accomodate the
selection of different infrastructures.  No .F90 files are changed but there are
small changes to ac/configure.ac to accomodate the new structure while also
allowing for the old target to use the old structure.  All answers are bitwise
identical, and all MOM6-examples and TC tests are passing.
  Updated the MOM6/docs files and .gitlab-ci.yml to reflect the new config_src
directory structure.
- Use a new branch of MRS in the gitlab pipelines
- The job within the gitlab pipeline gnu:ice-ocean-nolibs had a modified
  search path but a "," instead of a "/" led to al caps being compiled
  and thus failure.
This patch removes the AC_CHECK_FILE autoconf macro which assigns a
default path to the MOM_memory.h file path.

This path was defaulting back to the old directory, and was added to
support the regression verification testing, but this is a regression
problem with should be handled by the regression, not the autoconf
configuration.

This will produce a regression test fail, but it not a cause for concern.
Remove AC_CHECK_FILE for legacy MOM_memory.h
This patch removes two lines where the target build (used for regression
testing) were using the active branch's configure.ac, which caused path
mismatches.  We now use our local autoconf configuration files.

This patch also fixes a typo in the source code dependencies of the
target repository.

The AC_CHECK_FILE for the solo driver has also beed removed, for the
same reason as the MOM_memory.h check.

Some comments were also added or reformatted to undocumented flags.
Testing: Target uses local autoconf
@codecov
Copy link
Copy Markdown

codecov Bot commented Feb 26, 2021

Codecov Report

Merging #1342 (0ead900) into main (00c2819) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##             main    #1342   +/-   ##
=======================================
  Coverage   45.75%   45.75%           
=======================================
  Files         234      234           
  Lines       72542    72542           
=======================================
  Hits        33189    33189           
  Misses      39353    39353           
Impacted Files Coverage Δ
...g_src/drivers/solo_driver/MESO_surface_forcing.F90 0.00% <ø> (ø)
config_src/drivers/solo_driver/MOM_driver.F90 68.69% <ø> (ø)
...ig_src/drivers/solo_driver/MOM_surface_forcing.F90 26.54% <ø> (ø)
...fig_src/drivers/solo_driver/atmos_ocean_fluxes.F90 0.00% <ø> (ø)
...g_src/drivers/solo_driver/user_surface_forcing.F90 0.00% <ø> (ø)
config_src/infra/FMS1/MOM_coms_infra.F90 59.42% <ø> (ø)
config_src/infra/FMS1/MOM_couplertype_infra.F90 6.81% <ø> (ø)
config_src/infra/FMS1/MOM_cpu_clock_infra.F90 90.90% <ø> (ø)
config_src/infra/FMS1/MOM_data_override_infra.F90 0.00% <ø> (ø)
config_src/infra/FMS1/MOM_diag_manager_infra.F90 48.64% <ø> (ø)
... and 6 more

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 00c2819...e0bda29. Read the comment docs.

@kshedstrom
Copy link
Copy Markdown
Collaborator

I approve this PR. If this is for the whole consortium to evaluate, you should probably add other mentions.

@marshallward
Copy link
Copy Markdown
Collaborator Author

Thanks for the reminder, @kshedstrom.

Could the others please review?

@jiandewang
Copy link
Copy Markdown
Collaborator

@marshallward There are some directories naming changings, but there are no files adding/deleting or renaming, is that right ?

@marshallward
Copy link
Copy Markdown
Collaborator Author

@jiandewang Yes, that's correct. The new files were added in the previous PR to main.

@alperaltuntas
Copy link
Copy Markdown
Collaborator

I approve this PR.

Copy link
Copy Markdown
Collaborator

@abozec abozec left a comment

Choose a reason for hiding this comment

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

COAPS approves this PR.

@jiandewang
Copy link
Copy Markdown
Collaborator

@marshallward in EMC we are in final stage to merge your previous PR, so please bear me a bit delay on this PR.

@marshallward
Copy link
Copy Markdown
Collaborator Author

@jiandewang No hurry, please take as long as you need.

@jiandewang
Copy link
Copy Markdown
Collaborator

@jiandewang No hurry, please take as long as you need.

@thanks for your understanding

Copy link
Copy Markdown
Collaborator

@sanAkel sanAkel 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 can be merged. (No change in answers w.r.t. present version of GEOS coupled model with MOM6.)

If not already in place, I would also suggest/request an edit to the compile instructions at MOM6-examples: ocean-only, ice-ocean reflecting the reorganized directory structure.

@adcroft
Copy link
Copy Markdown
Collaborator

adcroft commented Mar 7, 2021

@sanAkel Thanks. Updated instructions have been written but not posted. Preview is at https://github.com/NOAA-GFDL/MOM6-examples/wiki/Temp---new-Getting-Started

@jiandewang
Copy link
Copy Markdown
Collaborator

@marshallward: works fine in UFS.

@marshallward
Copy link
Copy Markdown
Collaborator Author

OK, I will go ahead and merge this PR now.

Modes which need to use the legacy FMS library should be able to continue doing so, and ought not need any other major changes to their build process.

For those who need to upgrade to FMS2, we will start consolidating existing work into the FMS2 adapters and will update main as soon as possible.

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.

8 participants