Skip to content

Autoconf: Explicit MOM_memory.h configuration#366

Merged
Hallberg-NOAA merged 1 commit intoNOAA-GFDL:dev/gfdlfrom
marshallward:ac_mom_mem_config
May 29, 2023
Merged

Autoconf: Explicit MOM_memory.h configuration#366
Hallberg-NOAA merged 1 commit intoNOAA-GFDL:dev/gfdlfrom
marshallward:ac_mom_mem_config

Conversation

@marshallward
Copy link
Copy Markdown
Member

MOM6 requires an explicit MOM_memory.h header to define its numerical field memory layout. Previously, autoconf provided a flag to configure this with --enable-*, but was prone to two issues:

  • The binary choice of symmetric/nonsymmetric prevented use of static headers.

  • It was an incorrect use of --enable-*, which is intended to enable additional internal features; it is not used to select a mode.

To address these issues, we drop the flag and replace it with an AC_ARG_VAR variable, MOM_MEMORY, which is a path to the file. This variable will default to dynamic symmetric mode,

config_src/memory/dynamic_symmetric/MOM_memory.h

so there should be no change for existing users.

To the best of my knowledge, no one used the --enable-* flag, nor was it used in any automated systems (outside of .testing), so there should be no issue with dropping it.

.testing/Makefile was updated to use MOM_MEMORY.

@codecov
Copy link
Copy Markdown

codecov Bot commented May 14, 2023

Codecov Report

Merging #366 (8ded182) into dev/gfdl (0fa10ad) will decrease coverage by 0.01%.
The diff coverage is n/a.

❗ Current head 8ded182 differs from pull request most recent head 58ef026. Consider uploading reports for the commit 58ef026 to get more accurate results

@@             Coverage Diff              @@
##           dev/gfdl     #366      +/-   ##
============================================
- Coverage     38.13%   38.13%   -0.01%     
============================================
  Files           269      269              
  Lines         75743    75743              
  Branches      13926    13926              
============================================
- Hits          28883    28882       -1     
- Misses        41660    41662       +2     
+ Partials       5200     5199       -1     

see 1 file with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

MOM6 requires an explicit MOM_memory.h header to define its numerical
field memory layout.  Previously, autoconf provided a flag to configure
this with `--enable-*`, but was prone to two issues:

* The binary choice of symmetric/nonsymmetric prevented use of static
  headers.

* It was an incorrect use of `--enable-*`, which is intended to enable
  additional internal features; it is not used to select a mode.

To address these issues, we drop the flag and replace it with an
AC_ARG_VAR variable, MOM_MEMORY, which is a path to the file.  This
variable will default to dynamic symmetric mode,

    config_src/memory/dynamic_symmetric/MOM_memory.h

so there should be no change for existing users.

To the best of my knowledge, no one used the `--enable-*` flag, nor was
it used in any automated systems (outside of .testing), so there should
be no issue with dropping it.

.testing/Makefile was updated to use MOM_MEMORY.
Copy link
Copy Markdown
Member

@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.

These changes seem well-conceived and they have passed the pipeline testing at
https://gitlab.gfdl.noaa.gov/ogrp/MOM6/-/pipelines/19310 .

@Hallberg-NOAA Hallberg-NOAA merged commit 501fcff into NOAA-GFDL:dev/gfdl May 29, 2023
@marshallward marshallward deleted the ac_mom_mem_config branch July 21, 2023 14:40
dhruvbalwada pushed a commit to dhruvbalwada/MOM6 that referenced this pull request Jan 31, 2026
…GFDL#366)

* add the instance suffix to the correct part of the restart filename and do not re-add it later on

* Revert "add the instance suffix to the correct part of the restart filename and do not re-add it later on"

This reverts commit a749486.

* fix multiinstance restart filename
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.

2 participants