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

add gh/ci eamxx standalone testing #6637

Merged
merged 7 commits into from
Oct 4, 2024
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
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
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
- cov
mahf708 marked this conversation as resolved.
Show resolved Hide resolved
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
62 changes: 37 additions & 25 deletions components/eamxx/cmake/BuildCprnc.cmake
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
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
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
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
Loading