NOAA-PSL / land-offline_workflow

Creative Commons Zero v1.0 Universal
1 stars 9 forks source link

Adding new ecbuild-based cmake capability #17

Closed jkbk2004 closed 1 year ago

jkbk2004 commented 1 year ago

PR Instructions:

  1. Provide details under all headings below.
  2. Assign Clara and one other person as reviewers.
  3. If the PR is not ready for merging, add the "DRAFT/DO NOT MERGE" label.
  4. When a PR is ready to merge, remove the "DRAFT/DO NOT MERGE" and email Clara.
  5. Before requesting that the PR be merged, complete the checklist below.

Notes on preparing PR, using git can be found in README_git

Describe your changes

Summarise all code changes included in PR:

Adding new build feature with ecbuild-based cmake. The feature is comparable with current JEDI application build system. Build test worked well at top directory level

List any associated PRs in the submodules.

Issue ticket number and link

List the git Issue that this PR addresses:

Test output

Is this PR expected to pass the DA_IMS_test (ie., does it change the output)?

Does it pass the DA_IMS_test?

No need to test DA_IMS at this stage since still workflow script need get updated with new executables generated by cmake

If changes to the test results are expected, what are these changes? Provide a link to the output directory when running the test:

Checklist before requesting a review

jkbk2004 commented 1 year ago

@ClaraDraper-NOAA I finished all cleaning up and sanity check on hera. All works as expected. We can start merging in. Let me Need to merge PRs on submodules first. vector2tile, ufs_land_driver, IMS_proc, DA_update, and then land-offline_workflow on this PR. Sanity check experiment path on hera: /scratch2/NCEPDEV/marine/Jong.Kim/LandDA/test/land-offline_workflow-1

jkbk2004 commented 1 year ago

I finished all cleaning up and sanity check on hera. All works as expected. We can start merging in. Let me Need to merge PRs on submodules first. vector2tile, ufs_land_driver, IMS_proc, DA_update, and then land-offline_workflow on this PR. Sanity check experiment path on hera: /scratch2/NCEPDEV/marine/Jong.Kim/LandDA/test/land-offline_workflow-1

@ClaraDraper-NOAA let me know if you need me for final review again.

ClaraDraper-NOAA commented 1 year ago

thanks @jkbk2004 Can you please update the README with compilation instruction, including which modules need to be loaded?

Also, if I want to compile only a submodule (say the land-DA _update) is the process any different?

(sorry for the slow response, I was on leave and then IT locked me out of my computer for a few days).

jkbk2004 commented 1 year ago

thanks @jkbk2004 Can you please update the README with compilation instruction, including which modules need to be loaded?

Also, if I want to compile only a submodule (say the land-DA _update) is the process any different?

(sorry for the slow response, I was on leave and then IT locked me out of my computer for a few days).

updated readme. note that we need next pr to confirm jedi stack on hera. information might be available from GDASapp team. Additional features are possible inside cmakelist and more PRs can follow.

ClaraDraper-NOAA commented 1 year ago

Thanks Jong. With a few small changes I have it running. However, the test is failing, as you've forked the sub-modules off the head of each branch, rather than the hashes that develop is pointing to (and I'm mid-way through implementing some science changes in the code that applies the increments). For the ufs-land-driver I suspect you've forked off Mike's repo, and not mine (which is slightly older and has a small science change). I'll update my submodules so that each points to the head, and then update the tests and try again.

jkbk2004 commented 1 year ago

Thanks Jong. With a few small changes I have it running. However, the test is failing, as you've forked the sub-modules off the head of each branch, rather than the hashes that develop is pointing to (and I'm mid-way through implementing some science changes in the code that applies the increments). For the ufs-land-driver I suspect you've forked off Mike's repo, and not mine (which is slightly older and has a small science change). I'll update my submodules so that each points to the head, and then update the tests and try again.

Sure! let me know if you need anything. We are going to go ahead to add sphinx doc folder so that we can move on archiving a short user guide.

ClaraDraper-NOAA commented 1 year ago

@jkbk2004 I've updated my hashes for DA_update and vector2tile repos to point to the develop head. Can you please update your hashes to each? ( to solve the merge conflict above). I updated your repos.