NOAA-EMC / GDASApp

Global Data Assimilation System Application
GNU Lesser General Public License v2.1
14 stars 30 forks source link

restore ATM local ensemble ctest functionality #1003

Closed RussTreadon-NOAA closed 5 months ago

RussTreadon-NOAA commented 5 months ago

g-w develop commit 47302153 modified how EUPD_CYC is set in config.base. This variable is now set in a configuration yaml. This requires a change in how test_gdasapp_atm_jjob_ens is configured. This PR include the required change.

Fixes #1002

emcbot commented 5 months ago

Automated Global-Workflow GDASApp Testing Results: Machine: orion

Start: Thu Mar 28 10:35:16 CDT 2024 on Orion-login-1.HPC.MsState.Edu
---------------------------------------------------
Build:                                 *SUCCESS*
Build: Completed at Thu Mar 28 11:27:14 CDT 2024
---------------------------------------------------
Tests:                                  *Failed*
Tests: Failed at Thu Mar 28 11:53:07 CDT 2024
Tests: 94% tests passed, 3 tests failed out of 53
    1737 - test_gdasapp_fv3jedi_fv3inc (Failed)
    1771 - test_gdasapp_atm_jjob_var_run (Failed)
    1772 - test_gdasapp_atm_jjob_var_final (Failed)
Tests: see output at /work2/noaa/stmp/cmartin/CI/GDASApp/workflow/PR/1003/global-workflow/sorc/gdas.cd/build/log.ctest
RussTreadon-NOAA commented 5 months ago

The test_gdasapp_fv3jedi_fv3inc and test_gdasapp_atm_jjob_var_run failures appear to be related to GDASApp hash 18ba5da. Does this sound reasonable @DavidNew-NOAA ?

RussTreadon-NOAA commented 5 months ago

test_gdasapp_atm_jjob_ens Passed with the changes in this PR

50/53 Test #1773: test_gdasapp_atm_jjob_ens_init ........................   Passed   44.25 sec
      Start 1774: test_gdasapp_atm_jjob_ens_run
51/53 Test #1774: test_gdasapp_atm_jjob_ens_run .........................   Passed  330.24 sec
      Start 1775: test_gdasapp_atm_jjob_ens_final
52/53 Test #1775: test_gdasapp_atm_jjob_ens_final .......................   Passed   42.24 sec

As such, I think this PR is ready for review, approval, and merger into GDASApp develop.

RussTreadon-NOAA commented 5 months ago

@DavidNew-NOAA : I see g-w PR #2420. I assume that if I build RussTreadon-NOAA:feature/eupd_cyc inside a working copy of DavidNew-NOAA:feature/jediinc2fv3, the Failed tests in this PR should change to Passed. Does this sound right?

DavidNew-NOAA commented 5 months ago

@DavidNew-NOAA : I see g-w PR #2420. I assume that if I build RussTreadon-NOAA:feature/eupd_cyc inside a working copy of DavidNew-NOAA:feature/jediinc2fv3, the Failed tests in this PR should change to Passed. Does this sound right?

@RussTreadon-NOAA Yes, that's correct for the test_gdasapp_fv3jedi_fv3inc test. I assume that's the case for the test_gdasapp_atm_jjob_var_run, but I'm realizing now that I may have needed to modify those jjob tests in the my GDASApp PR, given that g-w PR #2420 changes the structure and names of two atmanl jobs. I need to investigate.

DavidNew-NOAA commented 5 months ago

OK, so JGLOBAL_ATM_ANALYSIS_RUN is now JGLOBAL_ATM_ANALYSIS_VARIATIONAL in g-w branch feature/jediinc2fv3 will break the test_gdasapp_atm_jjob_var_run test. I will need to do a quick bugfix PR into GDASApp develop. Thanks for feedback.

RussTreadon-NOAA commented 5 months ago

@DavidNew-NOAA , thanks for letting me know. Instead of you opening a new issue to fix ATM variational ctest functionality, I can modify the description of this issue and make the change in my branch. Does this work for you?

DavidNew-NOAA commented 5 months ago

@DavidNew-NOAA , thanks for letting me know. Instead of you opening a new issue to fix ATM variational ctest functionality, I can modify the description of this issue and make the change in my branch. Does this work for you?

@RussTreadon-NOAA Sure, that would be great, thank you. However, in addition do modifying the run job test, a new test will need to be made for the new job JGLOBAL_ATM_ANALYSIS_FV3_INCREMENT (gdasatmanlfv3inc), which is added in feature/jediinc2fv3.

RussTreadon-NOAA commented 5 months ago

@DavidNew-NOAA: OK, I see. I jumped before looking. While the changes for adding an new job step are straight forward, I may not get everything in place and tested before I go on leave COB tomorrow. Given this, it's preferable to limit this PR to test_atm_jjob_ens and open a new issue & PR to expand test_atm_jjob_var to with with the new job.

DavidNew-NOAA commented 5 months ago

Ok, sounds good.

RussTreadon-NOAA commented 5 months ago

@CoryMartin-NOAA , @guillaumevernieres , or @DavidNew-NOAA : would one of you review these changes and approve so we can merge this fix into GDASApp develop.

RussTreadon-NOAA commented 5 months ago

Thank you @CoryMartin-NOAA . Merging now.

DavidNew-NOAA commented 5 months ago

Hi Russ,

To clarify, do you want me to open this issue and PR since you'll be on leave?

David

On Thu, Mar 28, 2024 at 1:47 PM RussTreadon-NOAA @.***> wrote:

@DavidNew-NOAA https://github.com/DavidNew-NOAA: OK, I see. I jumped before looking. While the changes for adding an new job step are straight forward, I may not get everything in place and tested before I go on leave COB tomorrow. Given this, it's preferable to limit this PR to test_atm_jjob_ens and open a new issue & PR to expand test_atm_jjob_var to with with the new job.

— Reply to this email directly, view it on GitHub https://github.com/NOAA-EMC/GDASApp/pull/1003#issuecomment-2025786027, or unsubscribe https://github.com/notifications/unsubscribe-auth/BAAUIHHXGXQYCEMNPR4MWJ3Y2RJTPAVCNFSM6AAAAABFM74UBWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDAMRVG44DMMBSG4 . You are receiving this because you were mentioned.Message ID: @.***>

RussTreadon-NOAA commented 5 months ago

@DavidNew-NOAA : Yes, please open an issue to document the fact that the test_gdasapp_atm_jjob_var suite is now incomplete given the new fv3inc job.