Skip to content

+New config_src directory structure#1339

Merged
marshallward merged 8 commits into
mom-ocean:gfdl-fms2from
Hallberg-NOAA:gfdl-fms2
Feb 26, 2021
Merged

+New config_src directory structure#1339
marshallward merged 8 commits into
mom-ocean:gfdl-fms2from
Hallberg-NOAA:gfdl-fms2

Conversation

@Hallberg-NOAA
Copy link
Copy Markdown
Collaborator

Created the new config_src directory tree structure, as agreed upon at
https://github.com/NOAA-GFDL/MOM6/discussions/1286, to eventually accommodate
the selection of different infrastructures. No .F90 files are changed but there
are small changes to ac/configure.ac to accommodate 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.

This change will require modifications to various compile scripts.

MJHarrison-GFDL and others added 7 commits February 23, 2021 13:31
	- this addresses a bug in the apply_ALE_sponge routine which
  	had been passing zero values to the remapping routine for the
	target grid thicknesses.
…ate-2021-02-19

Dev gfdl main candidate 2021-02-19
  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.
@codecov
Copy link
Copy Markdown

codecov Bot commented Feb 25, 2021

Codecov Report

Merging #1339 (0d22f32) into gfdl-fms2 (45b3366) will increase coverage by 0.00%.
The diff coverage is 75.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           gfdl-fms2    #1339   +/-   ##
==========================================
  Coverage      45.74%   45.75%           
==========================================
  Files            234      234           
  Lines          72540    72542    +2     
==========================================
+ Hits           33187    33189    +2     
  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 7 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 45b3366...0d22f32. Read the comment docs.

Comment thread ac/configure.ac
# a compile-time mode, so this is not exactly being used as intended.
MEM_LAYOUT=${srcdir}/config_src/dynamic_symmetric
MEM_LAYOUT=${srcdir}/config_src/memory/dynamic_symmetric
AC_CHECK_FILE($MEM_LAYOUT, [MEM_LAYOUT=$MEM_LAYOUT], [MEM_LAYOUT=${srcdir}/config_src/dynamic_symmetric])
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I'm guessing this directory also needs to be updated.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

No, it is correct as written. The point here is that if the MEM_LAYOUT file does not exist, it is because this is the target directory that is still using the old directory structure. Because I know essentially nothing about autoconf, I spent much more time this morning figuring out how to make the TC make commands work with both the old and new directory structures than I did with actually moving the files around!

Copy link
Copy Markdown
Collaborator

@marshallward marshallward Feb 25, 2021

Choose a reason for hiding this comment

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

But it's looking in defaulting to a directory which ought to no longer exist. Is this being done to accommodate the regression test?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Without this line the TC regression tests were failing to compile; with it they do. Once we are no longer interested in comparing runs with potentially different directory structures, we can get rid of the ACC_CHECK_FILE line.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Then at the least there needs to be a comment explaining why this is here. It's a nonsense macro in the context of the repository. While we do provide a regression test, we should not leave around musty old content to accommodate it.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

As a point of reference, the very last PR to main also did not pass regression.

  Updated the MOM6/docs files and .gitlab-ci.yml to reflect the new config_src
directory structure.
@adcroft adcroft mentioned this pull request Feb 25, 2021
@marshallward marshallward merged commit 0d22f32 into mom-ocean:gfdl-fms2 Feb 26, 2021
@Hallberg-NOAA Hallberg-NOAA deleted the gfdl-fms2 branch July 30, 2021 18:03
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