add gh/ci eamxx standalone testing #6637

mahf708 · 2024-09-23T16:04:06Z

Adds support for running eamxx standalone testing in the gh/ci containers, for now only single-precision tests due to a mix of issues. Addresses two issues that have already been fixed in the scream fork. Adds flexibility to build cprnc in eamxx.

[BFB]

github-actions · 2024-09-23T16:06:10Z

PR Preview Action v1.4.8
🚀 Deployed preview to https://E3SM-Project.github.io/E3SM/pr-preview/pr-6637/
on branch `gh-pages` at 2024-09-28 19:40 UTC

components/eamxx/cmake/machine-files/ghci-oci.cmake

mahf708 · 2024-09-23T16:28:54Z

FYI, the eamxx build fails are due to #6635. These won't affect the standalone testing because it is "sp" (homme isn't supported there). This is part of the reason I am starting with sp only, but there's a second part and it is to do with the complexity of building and executing the other (bigger) test suites, with a weird (unexpected) fail in atm_proc. I will handle those at a later time.

.github/workflows/eamxx-gh-ci-standalone.yml

mahf708 · 2024-09-25T22:41:18Z

Status update: This PR is very close to being ready, in fact, it may pass all tests, but I'd like to revise the container again before we merge. The rationale: The standalone tests are pushing the container very close to its edge in terms of disk space, resulting in unpredictable fails. Plan A: reduce the space inside the container itself first (we have 13+ GB of data, but we need only ~2 GB for the standalone tests). Plan B: reduce the space in the runner and redesign how the container is run.

bartgol · 2024-09-25T22:55:04Z

What do you mean by "reduce the space in the runner"?

mahf708 · 2024-09-25T23:29:53Z

What do you mean by "reduce the space in the runner"?

The runner image (ubuntu-latest) comes preloaded with all sorts of random things that we can delete. Nominally, they guarantee ~15 GB of space, but in reality, there's more than 120 GB on these runners. It's just that a lot of complex stuff is preinstalled. You can view a list of stuff they have on these in docs like this https://github.com/actions/runner-images/blob/ubuntu22/20240922.1/images/ubuntu/Ubuntu2204-Readme.md. See below for the runner image vs the custom image. I propose to make space in the custom image first. If it doesn't work, I will make space outside of it, but that will likely mean I cannot use the custom image as written below...

  ci:
    runs-on: ubuntu-latest <--------------------------------------- runner image
    strategy:
      fail-fast: false
      matrix:
        test:
          - sp
          - opt
    container: 
      image: ghcr.io/e3sm-project/containers-ghci:ghci-0.1.2 <----- custom image

bartgol · 2024-09-25T23:46:59Z

Aren't the limits set on the container image? Why do we care about the runner?

mahf708 · 2024-09-26T00:46:02Z

Aren't the limits set on the container image? Why do we care about the runner?

I am not sure what you mean...

The limit comes from the hardware (bare-metal), which has the runner image (ubuntu-latest), which has the container image (ghci-container). In other words, ghci-container ⊆ ubuntu-latest ⊆ bare-metal. Inside the ghci-container, you can use as much as you want (i.e., you can run a multi-TB sim if there is space...)

bartgol · 2024-09-26T14:58:50Z

Aren't the limits set on the container image? Why do we care about the runner?

I am not sure what you mean...

The limit comes from the hardware (bare-metal), which has the runner image (ubuntu-latest), which has the container image (ghci-container). In other words, ghci-container ⊆ ubuntu-latest ⊆ bare-metal. Inside the ghci-container, you can use as much as you want (i.e., you can run a multi-TB sim if there is space...)

Ah, so we are not fitting on the actual machine where things end up running? I see. Re: container image size, I was thinking gh has a limit in size for images that one can upload, and that's it. Since the runner is not ours, I thought "there's no limit on our end". But yes, while we are not responsible for the runner size, the limit is hardware based.

Wow, 150GB for the runner, that's just nonsense. Why do they make them this big? Have you considered asking for alpine instead? We may have to install something at startup though, which may take some time... Alternatively, maybe a basic debian?

rljacob · 2024-09-26T16:23:05Z

Is this still a draft?

mahf708 · 2024-09-26T16:47:22Z

Is this still a draft?

Yes, I want to rework the container a bit so that the new tests can pass more reliably. The issue is running out of disk space (sometimes)

.github/workflows/eamxx-gh-ci-standalone.yml

mahf708 · 2024-09-29T16:13:22Z

Is this still a draft?

@rljacob, this is ready now (see notes below).

@bartgol, could you take another look?

Some notes:

this PR introduces changes in EAMxx that are already impl'ed in the scream repo (namely b33da8c for a cosp segfault and 89b7ade for pl semantics --- these two are isolated commits that hopefully won't conflict when merging the repos, but Jim can comment)
this PR changes a little bit about cprnc building in eamxx (with more todo later)
this PR introduces four different workflows that will run when eamxx or homme are touched (standalone in four flavors); the fpe and dbg flavors can take up to two hours, the build alone is ~20 mins, so these will be our longest tests (recall we have 6 hours on these runners, so we are far awar from timing out)
a todo item is to introduce concurrency like Luca suggested (add a concurrency check to gh workflows #6652)

bartgol

Just a minor comment, but not really road-blocking.

bartgol · 2024-09-30T18:05:11Z

.github/workflows/eamxx-gh-ci-standalone.yml

+        uses: actions/upload-artifact@v4
+        if: ${{ always() }}
+        with:
+          name: ${{ matrix.test }}


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

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

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.

bartgol · 2024-09-30T18:05:46Z

.gitmodules

-	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


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

Is this ready or still needs a change?

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?

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

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

Adds support for running eamxx standalone testing in the gh/ci containers, for now only single-precision tests due to a mix of issues. Addresses two issues that have already been fixed in the scream fork. Adds flexibility to build cprnc in eamxx. [BFB]

mahf708 added Testing Anything related to unit/system tests EAMxx PRs focused on capabilities for EAMxx labels Sep 23, 2024

mahf708 requested review from bartgol and jgfouca September 23, 2024 16:04

jgfouca reviewed Sep 23, 2024

View reviewed changes

components/eamxx/cmake/machine-files/ghci-oci.cmake Show resolved Hide resolved

jgfouca approved these changes Sep 23, 2024

View reviewed changes

bartgol reviewed Sep 23, 2024

View reviewed changes

.github/workflows/eamxx-gh-ci-standalone.yml Show resolved Hide resolved

.github/workflows/eamxx-gh-ci-standalone.yml Outdated Show resolved Hide resolved

.github/workflows/eamxx-gh-ci-standalone.yml Outdated Show resolved Hide resolved

mahf708 changed the title ~~add gh/ci eamxx standalone testing~~ [wip] add gh/ci eamxx standalone testing Sep 23, 2024

mahf708 mentioned this pull request Sep 24, 2024

fixes for eamxx running from e3sm repo for sycl changes #6636

Closed

mahf708 added 3 commits September 25, 2024 16:45

add gh/ci eamxx standalone testing

fac8382

fix pl semanitics

89b7ade

only test for eamxx, save logs

cdaa5c4

mahf708 force-pushed the mahf708/ig/standalone-ghci-oci branch from 782baa7 to cdaa5c4 Compare September 25, 2024 21:46

mahf708 changed the title ~~[wip] add gh/ci eamxx standalone testing~~ add gh/ci eamxx standalone testing Sep 25, 2024

mahf708 added the BFB PR leaves answers BFB label Sep 25, 2024

try new slimmed container

d6bf994

mahf708 commented Sep 28, 2024

View reviewed changes

.github/workflows/eamxx-gh-ci-standalone.yml Outdated Show resolved Hide resolved

mahf708 added 3 commits September 28, 2024 16:15

don't run coverage test

da9951a

add fpe test

0012b49

update cosp2 submod

b33da8c

mahf708 marked this pull request as ready for review September 29, 2024 01:26

mahf708 mentioned this pull request Sep 29, 2024

add a concurrency check to gh workflows #6652

Closed

mahf708 requested a review from bartgol September 30, 2024 16:36

bartgol approved these changes Sep 30, 2024

View reviewed changes

rljacob self-assigned this Sep 30, 2024

rljacob merged commit 6920f5e into master Oct 4, 2024
13 checks passed

rljacob deleted the mahf708/ig/standalone-ghci-oci branch October 4, 2024 16:16

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add gh/ci eamxx standalone testing #6637

add gh/ci eamxx standalone testing #6637

mahf708 commented Sep 23, 2024 •

edited

Loading

github-actions bot commented Sep 23, 2024 •

edited

Loading

mahf708 commented Sep 23, 2024

mahf708 commented Sep 25, 2024

bartgol commented Sep 25, 2024

mahf708 commented Sep 25, 2024 •

edited

Loading

bartgol commented Sep 25, 2024

mahf708 commented Sep 26, 2024

bartgol commented Sep 26, 2024

rljacob commented Sep 26, 2024

mahf708 commented Sep 26, 2024

mahf708 commented Sep 29, 2024 •

edited

Loading

bartgol left a comment

bartgol Sep 30, 2024

mahf708 Sep 30, 2024

bartgol Sep 30, 2024

bartgol Sep 30, 2024

rljacob Oct 3, 2024

mahf708 Oct 3, 2024

mahf708 Oct 3, 2024

rljacob Oct 3, 2024

add gh/ci eamxx standalone testing #6637

add gh/ci eamxx standalone testing #6637

Conversation

mahf708 commented Sep 23, 2024 • edited Loading

github-actions bot commented Sep 23, 2024 • edited Loading

mahf708 commented Sep 23, 2024

mahf708 commented Sep 25, 2024

bartgol commented Sep 25, 2024

mahf708 commented Sep 25, 2024 • edited Loading

bartgol commented Sep 25, 2024

mahf708 commented Sep 26, 2024

bartgol commented Sep 26, 2024

rljacob commented Sep 26, 2024

mahf708 commented Sep 26, 2024

mahf708 commented Sep 29, 2024 • edited Loading

bartgol left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mahf708 commented Sep 23, 2024 •

edited

Loading

github-actions bot commented Sep 23, 2024 •

edited

Loading

mahf708 commented Sep 25, 2024 •

edited

Loading

mahf708 commented Sep 29, 2024 •

edited

Loading