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

Bring jcb into GDASapp and use version of jcb not using submodules #1146

Merged
merged 6 commits into from
Jun 18, 2024

Conversation

danholdaway
Copy link
Contributor

@danholdaway danholdaway commented Jun 4, 2024

Bring jcb into the GDASapp repo and use the versions of jcb that do not use submodules:

There is a GW branch of the same name and I will make a PR there once this is merged. https://github.com/danholdaway/global-workflow/tree/feature/move_jcb

* develop:
  Update wcoss2.intel.lua (#1142)
  Add yaml file for prepobsaero task (#1141)
  Diffusion parameter yaml template (#1140)
  Adjust absolute float tolerance for variational and ensemble DA jjob tests (#1136)
  Insitu temp and salt (#1135)
  Adds letkf.yaml(.j2)  (#1134)
  Add optional testing block to 3dvar and lgetkf input YAMLs for jjobs (#1129)
  updates to build and run some ctests on WCOSS2 (#1122)
  Addin a GFSv17 ctest (#1130)
  Feature/rtofs in situ (#1110)
  update jcb-gdas hash to bring in satwnd yaml changes (#1128)
  Optionally run specified rocoto task as part of the ctests  (#1121)
  Add build capability to Gaea-C5 (#1101)
  Python Converter and Json for the ADPUPA BUFR DUMP (#1115)
  Revert "Fix ice water FV3 increment variable name and add a few more … (#1125)
  Satwnd and variable name-convention updates for jcb implementation in GDASApp (#1119)
  Add inverse distance weighting option to superob function (#1116)
  Resume the Marine Vrfy Task on ctests (#1107)
  Replicate the creation of the gw-ci cycling test (#1114)
  Added YAML, JSON, python files for assimilating LEOGEO satwinds (#1099)
  Add an OOPS-based application to recenter snow analysis (#1102)
  Fix ice water FV3 increment variable name and add a few more (#1112)
  Update hera.intel.lua (#1109)
CoryMartin-NOAA
CoryMartin-NOAA previously approved these changes Jun 4, 2024
* develop:
  Changes name of rossrad file for letkf (#1151)
  Adds staging jinja/yaml file for letkf task (#1137)
@RussTreadon-NOAA
Copy link
Contributor

@danholdaway , the changes in GDASApp branch feature/move_jcb have been tested in g-w PR #2665. Everything looks good. g-w PR #2655 has been approved by Cory, Rahul, and me. Another good.

However, I now see a problem.

g-w PR #2655 merges your forked g-w branch danholdaway:feature/move_jcb into g-w develop. The forked g-w feature/move_jcb currently points at gdas.cd @ 0cda73b. This is problematic. This hash is GDASApp branch feature/move_jcb.

I think we want g-w develop to point at GDASApp develop, right? If so, we need to merge this PR into develop and then update the gdas.cd hash in danholdaway:feature/move_jcb accordingly.

@danholdaway
Copy link
Contributor Author

danholdaway commented Jun 7, 2024

That is correct @RussTreadon-NOAA. But as soon as we merge GDASapp everything will be broken while we wait for the GW PR to be merged. Alternatively we can merge G-W and then GDASapp and then make another g-w PR changing the hash to develop. That’s the only way that everything continues to work. What do you think? Is that acceptable?

Copy link
Contributor

@RussTreadon-NOAA RussTreadon-NOAA left a comment

Choose a reason for hiding this comment

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

Tested as part of g-w #2565. Looks good.

Approve.

@RussTreadon-NOAA
Copy link
Contributor

@danholdaway , OK. I see your logic. I never thought of having g-w point at a GDASApp branch instead of develop. This works.
g-w gdas.cd points at a GDASApp hash. g-w doesn't care if that hash is GDASApp develop, a branch, or a tag.

Your approach allows us to bypass the sequential method I've been using: GDASApp first and then g-w.

g-w PR #2665 can move ahead as is. After this GDASApp PR is merged into GDASApp develop we can decide if we want to open a new g-w PR to point at the updated GDASApp develop or let one of our other GDASApp-based g-w PRs (e.g., #2654, #2641) do this.

@danholdaway
Copy link
Contributor Author

@RussTreadon-NOAA perhaps in this instance we merge down from g-w to gdas to jcb. Just because it takes longest to get something into g-w wheres we can more quickly merge the others. And in terms of the code pointed to in the hashes it would be equivalent

@CoryMartin-NOAA
Copy link
Contributor

if it's an unchanged squash commit, is the hash identical in the branch and after merged to develop?

@RussTreadon-NOAA
Copy link
Contributor

Works for me. Just want to get as many of your changes into develop as soon as possible.

@danholdaway
Copy link
Contributor Author

if it's an unchanged squash commit, is the hash identical in the branch and after merged to develop?

No because you have to squash all the commits with a new hash. But the code that gets run would be identical I think.

@danholdaway
Copy link
Contributor Author

danholdaway commented Jun 7, 2024

Lets say we merge g-w, then gdas then jcb. Then immediately merged upward jcb, gdas, g-w so that the hashes were the hashes develop points to for each repo...I think the latter would be a sequence of empty PRs. In the g-w case they might even reject the PR, as Rahul did when I tried to bump the hash of GSI utils despite no changes. I might actually try that to see if that theory is correct after these are merged. For non-breaking changes it wouldn't matter which way you merged.

@RussTreadon-NOAA
Copy link
Contributor

g-w PR #2665 was merged into g-w develop at 5af325a (6/13/2024). g-w PR #2665 points g-w gdas.cd at GDASApp branch feature/move_jcb at 368c9c5.

We should merge this PR into GDASApp develop and update the g-w gdas.cd hash to point at the updated GDASApp develop.

@RussTreadon-NOAA RussTreadon-NOAA added the hera-RT Queue for automated testing on Hera label Jun 18, 2024
@emcbot emcbot added hera-RT-Running Automated testing running on Hera and removed hera-RT Queue for automated testing on Hera labels Jun 18, 2024
@RussTreadon-NOAA RussTreadon-NOAA added the hera-GW-RT Queue for automated testing with global-workflow on Hera label Jun 18, 2024
@emcbot emcbot added hera-GW-RT-Running Automated testing with global-workflow running on Hera and removed hera-GW-RT Queue for automated testing with global-workflow on Hera labels Jun 18, 2024
@emcbot
Copy link

emcbot commented Jun 18, 2024

Automated GDASApp Testing Results:
Machine: hera

Start: Tue Jun 18 14:42:23 UTC 2024 on hfe03
---------------------------------------------------
Build:                                 *SUCCESS*
Build: Completed at Tue Jun 18 15:30:15 UTC 2024
---------------------------------------------------
Tests:                                 *SUCCESS*
Tests: Completed at Tue Jun 18 15:32:13 UTC 2024
Tests: 100% tests passed, 0 tests failed out of 24

@emcbot emcbot added hera-RT-Passed Automated testing successful on Hera and removed hera-RT-Running Automated testing running on Hera labels Jun 18, 2024
@emcbot
Copy link

emcbot commented Jun 18, 2024

Automated Global-Workflow GDASApp Testing Results:
Machine: hera

Start: Tue Jun 18 15:03:23 UTC 2024 on hfe03
---------------------------------------------------
Build:                                 *SUCCESS*
Build: Completed at Tue Jun 18 15:55:03 UTC 2024
---------------------------------------------------
Tests:                                 *SUCCESS*
Tests: Completed at Tue Jun 18 16:30:44 UTC 2024
Tests: 100% tests passed, 0 tests failed out of 48

@emcbot emcbot added hera-GW-RT-Passed Automated testing with global-workflow successful on Hera and removed hera-GW-RT-Running Automated testing with global-workflow running on Hera labels Jun 18, 2024
@RussTreadon-NOAA
Copy link
Contributor

Orion test
Install feature/move_jcb on Orion in g-w develop. Run test_gdasapp. 48 out of 48 tests pass

Test project /work2/noaa/da/rtreadon/git/global-workflow/test/sorc/gdas.cd/build
      Start 1489: test_gdasapp_util_coding_norms
 1/48 Test #1489: test_gdasapp_util_coding_norms ........................   Passed    5.24 sec
      Start 1490: test_gdasapp_util_ioda_example
 2/48 Test #1490: test_gdasapp_util_ioda_example ........................   Passed    4.55 sec

...

      Start 1869: test_gdasapp_atm_jjob_ens_final
47/48 Test #1869: test_gdasapp_atm_jjob_ens_final .......................   Passed   42.23 sec
      Start 1870: test_gdasapp_aero_gen_3dvar_yaml
48/48 Test #1870: test_gdasapp_aero_gen_3dvar_yaml ......................   Passed    0.77 sec

100% tests passed, 0 tests failed out of 48

Label Time Summary:
gdas-utils    =  17.38 sec*proc (11 tests)
script        =  17.38 sec*proc (11 tests)

Total Test time (real) = 1675.41 sec

@RussTreadon-NOAA
Copy link
Contributor

@guillaumevernieres , given successful CI on Hera (automated tests) and Orion, I'd like to merge this PR into develop. Any objections?

@guillaumevernieres
Copy link
Contributor

@guillaumevernieres , given successful CI on Hera (automated tests) and Orion, I'd like to merge this PR into develop. Any objections?

sounds good @RussTreadon-NOAA

@RussTreadon-NOAA
Copy link
Contributor

This PR has two companion PRs

Both have already been approved by @CoryMartin-NOAA. Given this, we should merge these jcb PRs as part of merging this PR. Note that feature/move_jcb already points at the appropriate jcb hashes so we don't need to update jcb hashes used by this PR.

@RussTreadon-NOAA RussTreadon-NOAA merged commit 7d2fe30 into develop Jun 18, 2024
17 checks passed
@RussTreadon-NOAA RussTreadon-NOAA deleted the feature/move_jcb branch June 18, 2024 18:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
hera-GW-RT-Passed Automated testing with global-workflow successful on Hera hera-RT-Passed Automated testing successful on Hera
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants