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

ifs-source sync #8

Merged
merged 131 commits into from
Jan 25, 2024
Merged

ifs-source sync #8

merged 131 commits into from
Jan 25, 2024

Conversation

awnawab
Copy link
Contributor

@awnawab awnawab commented Dec 28, 2023

This PR is one part of a two-way sync between ifs-source contrib/ecwam and the standalone repo. The corresponding ifs-source branch is:
https://git.ecmwf.int/users/naan/repos/ifs-source/commits?until=refs%2Fheads%2Fnaan_CY49R1_ecwam_sync.IFS-3196

The code herein is string-identical to the ifs-source branch, which has bit-identical results in both double and single precision to the CY49R1 CI forecast experiment.

The PR comprises mostly of the scientific changes for CY49. The validation hashes for all the test cases have been updated accordingly. Some technical changes are also included:

  • Updated runner scripts to reflect the updated namelist inputs in CY49
  • Fix to the FIELD_API fetchcontent mechanism to pick up FIELD_API in ifs-source/contrib
  • Optimization downgrade for propconnect.F90 for Intel builds
  • Disable dependence on ecflow_light in standalone ecWAM builds
  • LUMI-G communication hang fix contributed by Ioan Hadade
  • NVHPC optimization downgrade for sbottom.F90 contributed by Michael Lange

@FussyDuck
Copy link

FussyDuck commented Dec 28, 2023

CLA assistant check
All committers have signed the CLA.

@awnawab awnawab force-pushed the naan-ifs-source-sync branch from b97b16f to 3f77924 Compare December 29, 2023 00:04
@awnawab
Copy link
Contributor Author

awnawab commented Dec 29, 2023

@ioanhadade and @jrbidlot could you please sign the CLA? This PR contains commits contributed by both of you. @jrbidlot as most of the commits included herein are yours, could you also please review the PR? Thanks!

@awnawab awnawab requested a review from wdeconinck December 29, 2023 09:21
CMakeLists.txt Outdated
set(ECWAM_STANDALONE 0)
ecbuild_find_package( ecflow_light REQUIRED )
endif()

Copy link
Collaborator

Choose a reason for hiding this comment

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

This could be replaced with

ecbuild_add_option( FEATURE ECFLOW
                    DESCRIPTION "ecflow meter updates"
                    REQUIRED_PACKAGES )

It is then not an absolutely required package unless requested via "-DENABLE_ECFLOW=ON" or in the bundle.yml.
This then does not bind this feature to a package or bundle name.

@@ -414,7 +423,7 @@ endif()
ecbuild_add_library(
TARGET ${ecwam}
SOURCES ${ecwam_srcs}
PUBLIC_LIBS fiat parkind_${prec} ${ecwam}_intfb
PUBLIC_LIBS fiat parkind_${prec} $<$<NOT:${ECWAM_STANDALONE}>:ecflow_lightf> ${ecwam}_intfb
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
PUBLIC_LIBS fiat parkind_${prec} $<$<NOT:${ECWAM_STANDALONE}>:ecflow_lightf> ${ecwam}_intfb
PUBLIC_LIBS fiat parkind_${prec} $<${HAVE_ECFLOW}:ecflow_lightf> ${ecwam}_intfb

@@ -423,7 +432,7 @@ ecbuild_add_library(
field_api_${prec}
PUBLIC_INCLUDES $<INSTALL_INTERFACE:include>
PRIVATE_INCLUDES ${${PNAME}_OCEANMODEL_INCLUDE_DIRS}
PUBLIC_DEFINITIONS ${ECWAM_DEFINITIONS}
PUBLIC_DEFINITIONS ${ECWAM_DEFINITIONS} $<${ECWAM_STANDALONE}:_ECWAM_STANDALONE>
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can the logic of _ECWAM_STANDALONE be changed to PRIVATELY define variable WAM_HAVE_ECFLOW
and use that within the code?
This is consistent with other preprocessor definitions.

@ioanhadade
Copy link
Contributor

ioanhadade commented Jan 8, 2024 via email

@awnawab awnawab force-pushed the naan-ifs-source-sync branch from f0295f5 to b326f5a Compare January 9, 2024 11:14
@wdeconinck wdeconinck merged commit fcadfe8 into main Jan 25, 2024
9 checks passed
@awnawab awnawab deleted the naan-ifs-source-sync branch February 8, 2024 22:34
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