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.

Sign up for GitHub

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

add gh/ci eamxx standalone testing #6637

Merged
merged 7 commits into from
Oct 4, 2024
Merged

add gh/ci eamxx standalone testing #6637

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
fac8382
add gh/ci eamxx standalone testing
mahf708 Sep 23, 2024
89b7ade
fix pl semanitics
mahf708 Sep 24, 2024
cdaa5c4
only test for eamxx, save logs
mahf708 Sep 24, 2024
d6bf994
try new slimmed container
mahf708 Sep 28, 2024
da9951a
don't run coverage test
mahf708 Sep 28, 2024
0012b49
add fpe test
mahf708 Sep 28, 2024
b33da8c
update cosp2 submod
mahf708 Sep 28, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
55 changes: 55 additions & 0 deletions .github/workflows/eamxx-gh-ci-standalone.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
name: gh-standalone

on:
pull_request:
branches: [ master ]
mahf708 marked this conversation as resolved.
Show resolved Hide resolved
paths:
# first, yes to these
- '.github/workflows/eamxx-gh-ci-standalone.yml'
- 'cime_config/machine/config_machines.xml'
- 'components/eamxx/**'
- 'components/homme/**'
# second, no to these
- '!components/eamxx/docs/**'
- '!components/eamxx/mkdocs.yml'

workflow_dispatch:

jobs:

ci:
runs-on: ubuntu-latest
strategy:
fail-fast: false
matrix:
test:
- sp
- opt
- dbg
- fpe
container:
image: ghcr.io/e3sm-project/containers-standalone-ghci:standalone-ghci-0.1.0

steps:
-
name: Checkout
uses: actions/checkout@v4
with:
show-progress: false
submodules: recursive
-
name: standalone
env:
SHELL: sh
run: |
# TODO: get rid of this extra line if we can?
git config --global safe.directory '*'
./components/eamxx/scripts/test-all-scream -m ghci-oci -t ${{ matrix.test }}
-
name: Artifacts
uses: actions/upload-artifact@v4
if: ${{ always() }}
with:
name: ${{ matrix.test }}
Copy link
Contributor

Choose a reason for hiding this comment

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

May I suggest a name that is a bit longer, so the user knows what they will be downloading? Something like ctest-logs-${{ matrix.test }}?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Let's touch up this in the next iteration of edits. We need to establish some standards, I think :) I can explain why I chose short names everywhere (e.g., gh/ci(...) and gh-standalone/ci(opt)) --- mainly for ease of seeing them, on the actions boards... but maybe that's just a minor personal preference...

Copy link
Contributor

Choose a reason for hiding this comment

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

I am ok with short names for actions names, since they appear at the bottom of the PR page, and long names are problematic. But artifacts are in the action page, and there's plenty of space for a longer name. That said, I think it's very subjective, hence the approval regardless.

path: |
components/eamxx/ctest-build/*/Testing/Temporary/Last*.log
4 changes: 2 additions & 2 deletions .gitmodules
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,8 @@
branch = scorpio_classic
[submodule "cosp2"]
path = components/eam/src/physics/cosp2/external
url = [email protected]:CFMIP/COSPv2.0.git
branch = CESM_v2.1.4
url = [email protected]:bartgol/COSPv2.0.git
branch = bartgol/fix-cosp_optical_inputs
Copy link
Contributor

Choose a reason for hiding this comment

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

I have to change this. That branch got merged into the main cosp branch...

Copy link
Member

Choose a reason for hiding this comment

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

Is this ready or still needs a change?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think it is ready because the change hasn't been in SCREAM yet. If you prefer, I can make the change in both places?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@rljacob for reference, this is scream master right now https://github.com/E3SM-Project/scream/blob/a269ef91a3da65ee4e23f4e33736684366046b4d/.gitmodules#L23-L26 (https://github.com/E3SM-Project/scream/blob/master/.gitmodules#L23-L26)

Copy link
Member

Choose a reason for hiding this comment

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

If its not really necessary for this PR then no need to change it.

[submodule "cime"]
path = cime
url = [email protected]:ESMCI/cime.git
Expand Down
2 changes: 1 addition & 1 deletion components/eam/src/physics/cosp2/external
Open in desktop
Submodule external updated 2 files
+1 −0 driver/src/cosp2_test.f90
+1 −1 src/cosp.F90
62 changes: 37 additions & 25 deletions components/eamxx/cmake/BuildCprnc.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,32 +8,44 @@
include (EkatUtils)
macro(BuildCprnc)

# Make sure this is built only once
if (NOT TARGET cprnc)
if (SCREAM_CIME_BUILD)
string (CONCAT MSG
"WARNING! By default, scream should not build tests in a CIME build,\n"
"and cprnc should only be built by scream in case tests are enabled.\n"
"If you explicitly requested tests to be on in a CIME build,\n"
"then you can discard this warning. Otherwise, please, contact developers.\n")
message("${MSG}")
endif()
set(BLDROOT ${PROJECT_BINARY_DIR}/externals/cprnc)
file(WRITE ${BLDROOT}/Macros.cmake
"
set(SCC ${CMAKE_C_COMPILER})
set(SFC ${CMAKE_Fortran_COMPILER})
set(FFLAGS \"${CMAKE_Fortran_FLAGS}\")
set(NETCDF_PATH ${NetCDF_Fortran_PATH})
"
)
set(SRC_ROOT ${SCREAM_BASE_DIR}/../..)
add_subdirectory(${SRC_ROOT}/cime/CIME/non_py/cprnc ${BLDROOT})
EkatDisableAllWarning(cprnc)

set(CPRNC_BINARY ${BLDROOT}/cprnc CACHE INTERNAL "")

# TODO: handle this more carefully and more gracefully in the future
# TODO: For now, it is just a hack to get going...
# find cprnc defined in machine entries
set(CCSM_CPRNC $ENV{CCSM_CPRNC})
if(EXISTS "${CCSM_CPRNC}")
message(STATUS "Path ${CCSM_CPRNC} exists, so we will use it")
set(CPRNC_BINARY ${CCSM_CPRNC} CACHE INTERNAL "")
configure_file (${SCREAM_BASE_DIR}/cmake/CprncTest.cmake.in
${CMAKE_BINARY_DIR}/bin/CprncTest.cmake @ONLY)
else()
message(WARNING "Path ${CCSM_CPRNC} does not exist, so we will try to build it")
# Make sure this is built only once
if (NOT TARGET cprnc)
if (SCREAM_CIME_BUILD)
string (CONCAT MSG
"WARNING! By default, scream should not build tests in a CIME build,\n"
"and cprnc should only be built by scream in case tests are enabled.\n"
"If you explicitly requested tests to be on in a CIME build,\n"
"then you can discard this warning. Otherwise, please, contact developers.\n")
message("${MSG}")
endif()
set(BLDROOT ${PROJECT_BINARY_DIR}/externals/cprnc)
file(WRITE ${BLDROOT}/Macros.cmake
"
set(SCC ${CMAKE_C_COMPILER})
set(SFC ${CMAKE_Fortran_COMPILER})
set(FFLAGS \"${CMAKE_Fortran_FLAGS}\")
set(NETCDF_PATH ${NetCDF_Fortran_PATH})
"
)
set(SRC_ROOT ${SCREAM_BASE_DIR}/../..)
add_subdirectory(${SRC_ROOT}/cime/CIME/non_py/cprnc ${BLDROOT})
EkatDisableAllWarning(cprnc)

set(CPRNC_BINARY ${BLDROOT}/cprnc CACHE INTERNAL "")

configure_file (${SCREAM_BASE_DIR}/cmake/CprncTest.cmake.in
${CMAKE_BINARY_DIR}/bin/CprncTest.cmake @ONLY)
endif()
endif()
endmacro()
13 changes: 13 additions & 0 deletions components/eamxx/cmake/machine-files/ghci-oci.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,2 +1,15 @@
include(${CMAKE_CURRENT_LIST_DIR}/common.cmake)
common_setup()

set(CMAKE_Fortran_FLAGS "-Wno-maybe-uninitialized -Wno-unused-dummy-argument -fallow-argument-mismatch" CACHE STRING "" FORCE)
set(CMAKE_CXX_FLAGS "-fvisibility-inlines-hidden -fmessage-length=0 -Wno-use-after-free -Wno-unused-variable -Wno-maybe-uninitialized" CACHE STRING "" FORCE)

# TODO: figure out a better way to handle this, e.g.,
# TODO: --map-by ppr:1:node:pe=1 doesn't work with mpich,
# TODO: but -map-by core:1:numa:hwthread=1 may work well?
# TODO: this will need to be handled in EKAT at some point
set(EKAT_MPI_NP_FLAG "-np" CACHE STRING "-np")

# TODO: hack in place to get eamxx to recognize CPRNC
# TODO: See note in BuildCprnc.cmake...
set(ENV{CCSM_CPRNC} "/usr/local/packages/bin/cprnc")
mahf708 marked this conversation as resolved.
Show resolved Hide resolved
4 changes: 4 additions & 0 deletions components/eamxx/scripts/machines_specs.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,10 @@
["mpicxx","mpifort","mpicc"],
"",
""),
"ghci-oci" : ([f"eval $({CIMEROOT}/CIME/Tools/get_case_env -c SMS.ne4pg2_ne4pg2.F2010-SCREAMv1.ghci-oci_gnu)"],
["mpicxx","mpifort","mpicc"],
"",
""),
"linux-generic" : ([],["mpicxx","mpifort","mpicc"],"", ""),
"linux-generic-debug" : ([],["mpicxx","mpifort","mpicc"],"", ""),
"linux-generic-serial" : ([],["mpicxx","mpifort","mpicc"],"", ""),
Expand Down
2 changes: 1 addition & 1 deletion components/eamxx/src/share/tests/atm_process_tests.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -443,7 +443,7 @@ TEST_CASE("atm_proc_dag", "") {

using strvec_t = std::vector<std::string>;
auto params = create_test_params();
auto p1 = params.sublist("BarBaz");
auto& p1 = params.sublist("BarBaz");

// Make sure there's a missing piece (whatever Baz computes);
p1.set<strvec_t>("atm_procs_list",{"Bar"});
Expand Down