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 more CI to GW-CI #1365

Merged
merged 75 commits into from
Nov 25, 2024
Merged

Add more CI to GW-CI #1365

merged 75 commits into from
Nov 25, 2024

Conversation

DavidNew-NOAA
Copy link
Collaborator

@DavidNew-NOAA DavidNew-NOAA commented Nov 8, 2024

This PR is a companion to GW PR #3120.

It does a couple things:

  1. 5 GW CI tests are added/extended as CTests in GDASApp, running through to the fcst jobs in the first full-cycle. These CI tests are: C96C48_hybatmDA, C96C48_ufs_hybatmDA, C96C48_hybatmaerosnowDA, C48mx500_3DVarAOWCDA, and C48mx500_hybAOWCDA.
  2. Test references are added for C96C48_ufs_hybatmDA and C48mx500_3DVarAOWCDA, so that we're actually testing the output.
  3. These CTests are turned on by default in a workflow build, rather than having to mess with the CMakeCache.txt file and re-running make. This will allow us to use these tests in nightly testing.
  4. test/gw-ci/CMakeLists.txt is refactored quite a bit.
  5. There are 89 CTests, but for 5 CI tests, but I added task dependencies, so they can be run in parallel.

The primary motivation for this PR is that we can run CI for our nightly testing of GDASApp. Also, anyone with a PR can easily do CI testing through CTests.

@DavidNew-NOAA
Copy link
Collaborator Author

DavidNew-NOAA commented Nov 24, 2024

@CoryMartin-NOAA @RussTreadon-NOAA @danholdaway @guillaumevernieres

OK, I think this PR is now ready for final review and merger.

I made some additional changes:

  1. C48mx500_3DVarAOWCDA now has a test reference for marineanlvar
  2. I added CTest dependencies for the atm_jjob tests so they can be run in parallel.
  3. The locations of test references and test output files in GDASApp are centralized (test/testreference and build/gdas/test/testoutput respectively) to make updating references a bit easier. This mimics what the various JEDI repos do.
  4. I removed the old jediinc2fv3jedi.py script (which meant to delete a long time ago) and removed its CTest. I also removed the standalone CTest for gdas_fv3jedi_fv3inc.x since that application already tested twice in the atm_jjob and CI tests.
  5. The nightly testing scripts will use 12 processors to run the CTests to take advantage of the new ability to parallelize the tests.

ci/run_ci.sh Outdated Show resolved Hide resolved
ci/run_gw_ci.sh Outdated Show resolved Hide resolved
test/atm/global-workflow/CMakeLists.txt Show resolved Hide resolved
test/gw-ci/CMakeLists.txt Show resolved Hide resolved
test/testreference/C96C48_ufs_hybatmDA_fv3inc.ref Outdated Show resolved Hide resolved
test/testreference/C96C48_ufs_hybatmDA_lgetkf.ref Outdated Show resolved Hide resolved
Copy link
Contributor

@guillaumevernieres guillaumevernieres left a comment

Choose a reason for hiding this comment

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

Thanks @DavidNew-NOAA , looks good.

danholdaway
danholdaway previously approved these changes Nov 25, 2024
Copy link
Contributor

@danholdaway danholdaway left a comment

Choose a reason for hiding this comment

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

Thanks @DavidNew-NOAA. How long does it take to run all the ctests now? Not including the code building?

@DavidNew-NOAA
Copy link
Collaborator Author

@danholdaway Let me run them all one more time to give you an exact answer

@DavidNew-NOAA
Copy link
Collaborator Author

DavidNew-NOAA commented Nov 25, 2024

Let me run the test_gdasapp tests one more time, just for certainty. Then I'll merge

@DavidNew-NOAA
Copy link
Collaborator Author

I broke something somewhere, causing all test_gdasapp tests to fail. working on it

@DavidNew-NOAA DavidNew-NOAA merged commit 7a01c5e into develop Nov 25, 2024
5 checks passed
@DavidNew-NOAA DavidNew-NOAA deleted the feature/gw-ci branch November 25, 2024 23:35
@DavidNew-NOAA
Copy link
Collaborator Author

@danholdaway My last run of the 119 test_gdasapp CTests took 1 hour and 23 minutes

Comment on lines +391 to +392
set(pslot "GFSv17-3DVAR-C384mx025")
set(YAML_PATH ${HOMEgfs}/ci/cases/gfsv17/${pslot}.yaml)
Copy link
Collaborator

Choose a reason for hiding this comment

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

I am trying to run this set of tests, and it can't find the GFSv17-3DVAR-C384mx025.yaml in global-workflow/ci/cases/gfsv17. I don't see this file on develop, and I don't see it in NOAA-EMC/global-workflow#3120. What am I missing? (I have not yet tried to run with NOAA-EMC/global-workflow#3120)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@shlyaeva Yeah, that was a mistake on my part. I assumed the GFSv17 test name had the same naming convention as the low resolution marine CI. Thanks for pointing this out, I'll make a bugfix PR tomorrow morning.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Oh good, thank you for the clarification! I am still figuring out how to run things, so I wasn't sure whether I missed something.

WalterKolczynski-NOAA pushed a commit to NOAA-EMC/global-workflow that referenced this pull request Dec 12, 2024
…est (#3120)

This PR is a companion to NOAA-EMC/GDASApp#1365

It turns ```C96C48_ufs_hybatmDA``` and ```C48mx500_3DVarAOWCDA``` into a
regression test using the JEDI application testing feature. This feature
is turned on using the new ```DO_TEST_MODE``` parameter added to
```config.base``` in GW PR
[#3115](#3115). This
parameter is set to ```"YES"``` in the yaml defaults for the JEDI-based
CI tests in GW.

The motivation for this PR is a need to catch changes in JEDI which
alter the outputs of our applications.
---------

Co-authored-by: CoryMartin-NOAA <[email protected]>
Co-authored-by: RussTreadon-NOAA <[email protected]>
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.

6 participants