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

Autoconf: Explicit MOM_memory.h configuration #366

Merged
merged 1 commit into from
May 29, 2023

Conversation

marshallward
Copy link
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

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