Skip to content

Add the CMEPS based HAFS-HYCOM coupling into the HAFS workflow#38

Merged
BinLiu-NOAA merged 17 commits into
developfrom
feature/hafs_couplehycom
Feb 5, 2021
Merged

Add the CMEPS based HAFS-HYCOM coupling into the HAFS workflow#38
BinLiu-NOAA merged 17 commits into
developfrom
feature/hafs_couplehycom

Conversation

@BinLiu-NOAA
Copy link
Copy Markdown
Collaborator

@BinLiu-NOAA BinLiu-NOAA commented Jan 27, 2021

Bring in the CMEPS based HAFS-HYCOM coupling by merging the feature/hafs_couplehycom branch into the develop branch

  • Update hafs_forecast.fd to point to the latest support/HAFS branch, which now has the CMEPS based HYCOM coupling capability developed by @danrosen25, et al.
  • Update the HAFS workflow to support the CMEPS-based HYCOM coupling. The cpl_ocean configuration option is to control the different coupling methods in the workflow.
    - 0, run side-by-side forecast without coupling
    - 1, direct NUOPC/ESMF coupling through nearest point regridding
    - 2, direct NUOPC/ESMF coupling through bilinear regridding
    - 3, CMEPS based coupling through bilinear regridding
  • Add a new CCPP suite definition file: suite_HAFS_v0_gfdlmp_nonsst.xml

Note: The cpl_ocean option 3 currently gets slightly different results from the cpl_ocean option 2, which is caused by the term ordering differences in CMEPS resulting in small roundoff differences that build each coupling time step. But, @danrosen25 was able to reproduce bit-for-bit results if using a different branch of ESMF and turning on some regridding options that affect term ordering. Overall, it's just the butterfly effect, with about a 10^-14 difference during each regridding call.

@danrosen25
Copy link
Copy Markdown

Hi @BinLiu-NOAA,
Let's hold off on this . I have an open and nearly complete Pull Request to bring the HAFS-CMEPS development back to ESCOMP (NCAR is the owner of CMEPS). Once this goes through then we should update the revision of CMEPS to the ESCOMP:master branch.

@BinLiu-NOAA
Copy link
Copy Markdown
Collaborator Author

Hi @BinLiu-NOAA,
Let's hold off on this . I have an open and nearly complete Pull Request to bring the HAFS-CMEPS development back to ESCOMP (NCAR is the owner of CMEPS). Once this goes through then we should update the revision of CMEPS to the ESCOMP:master branch.

Hi @danrosen25, Sound great. Please let us know once the CMEPS pull request is done. We will update this pull request to use the latest CMEPS's support/HAFS branch as well. Thanks! Bin

…port/HAFS

branch of CMEPS. And a new ccpp SDF suite_HAFS_v0_gfdlmp_nonsst.xml is added.
@BinLiu-NOAA
Copy link
Copy Markdown
Collaborator Author

Hi @danrosen25,

I saw the CMEPS Pull Request has been merged. And I have update this feature/hafs_coupledhycom branch to use the latest support/HAFS branch accordingly. In the meantime, a new CCPP suite definition file: suite_HAFS_v0_gfdlmp_nonsst.xml is added, which will be used in hafsv0.2 baseline experiment.

So @uturuncoglu, @danrosen25, @BijuThomas-NOAA, @ZhanZhang-NOAA, @hyunsookkim-NOAA, @evankalina, please help to review this PR request when you get a chance.

Thanks!

Bin

P.S, @BijuThomas-NOAA could you please post the wall clock time difference here between the two experiments you conducted with the direct coupling and CMEPS based coupling? Thanks!

@BijuThomas-NOAA
Copy link
Copy Markdown
Contributor

BijuThomas-NOAA commented Jan 29, 2021 via email

@danrosen25
Copy link
Copy Markdown

danrosen25 commented Jan 29, 2021

I ran 5 debug queue tests on Hera. They each completed 12-15 hours of the simulation.
Note 1: These weren't bit-for-bit tests, which should be done through the regression test system.
Note 2: These weren't workflow tests. I'm doing those now.

Build / Run Configurations

  1. Build FV3ATM+HYCOM+CMEPS / Run FV3ATM+HYCOM+CMEPS
  2. Build FV3ATM+HYCOM+CMEPS / Run FV3ATM+HYCOM
  3. Build FV3ATM+HYCOM+CMEPS / Run FV3ATM
  4. Build FV3ATM+HYCOM / Run FV3ATM+HYCOM
  5. Build FV3ATM / Run FV3ATM

Component Revisions

  • 45a64f2e7396cdaadea21a02838c0c5e7123e68c sorc/hafs_forecast.fd
  • 19ff711593282f1e20afbbb8b4bda944ed058344 sorc/hafs_forecast.fd/CMEPS-interface/CMEPS
  • 27ad030c97fb285b2937c9abb6b4a28c54107a13 sorc/hafs_forecast.fd/FV3
  • 4c54010b0fe6ae67c20f7071d6df5b0e636b6113 sorc/hafs_forecast.fd/HYCOM-interface/HYCOM
  • a8a9c66c92a83539829e96e98a2de8d00da58118 sorc/hafs_forecast.fd/NEMS

@BinLiu-NOAA
Copy link
Copy Markdown
Collaborator Author

I ran 5 debug queue tests on Hera. They each completed 12-15 hours of the simulation.
Note 1: These weren't bit-for-bit tests, which should be done through the regression test system.
Note 2: These weren't workflow tests. I'm doing those now.

Build / Run Configurations

  1. Build FV3ATM+HYCOM+CMEPS / Run FV3ATM+HYCOM+CMEPS
  2. Build FV3ATM+HYCOM+CMEPS / Run FV3ATM+HYCOM
  3. Build FV3ATM+HYCOM+CMEPS / Run FV3ATM
  4. Build FV3ATM+HYCOM / Run FV3ATM+HYCOM
  5. Build FV3ATM / Run FV3ATM

Component Revisions

  • 45a64f2e7396cdaadea21a02838c0c5e7123e68c sorc/hafs_forecast.fd
  • 19ff711593282f1e20afbbb8b4bda944ed058344 sorc/hafs_forecast.fd/CMEPS-interface/CMEPS
  • 27ad030c97fb285b2937c9abb6b4a28c54107a13 sorc/hafs_forecast.fd/FV3
  • 4c54010b0fe6ae67c20f7071d6df5b0e636b6113 sorc/hafs_forecast.fd/HYCOM-interface/HYCOM
  • a8a9c66c92a83539829e96e98a2de8d00da58118 sorc/hafs_forecast.fd/NEMS

Hi @danrosen25, Thanks for conducting these tests! BTW, could you please point me to one of your coupled FV3ATM+HYCOM test run directory? I would like to check if the variables exchange between the two components successfully. Thanks! -Bin

@danrosen25
Copy link
Copy Markdown

@BinLiu-NOAA @uturuncoglu
You're right ... and this is a good reason for bit-for-bit tests. HYCOM isn't advertising any import or export fields by default anymore. We have to look into the code here.
https://github.com/hafs-community/HYCOM-src/blob/support/HAFS/NUOPC/HYCOM_OceanComp.F90#L711

Coupling with CMEPS is working because the fields are explicitly configured in nems.configure.

@BinLiu-NOAA
Copy link
Copy Markdown
Collaborator Author

BinLiu-NOAA commented Jan 30, 2021

@BinLiu-NOAA @uturuncoglu
You're right ... and this is a good reason for bit-for-bit tests. HYCOM isn't advertising any import or export fields by default anymore. We have to look into the code here.
https://github.com/hafs-community/HYCOM-src/blob/support/HAFS/NUOPC/HYCOM_OceanComp.F90#L711

Coupling with CMEPS is working because the fields are explicitly configured in nems.configure.

Thanks, @danrosen25! Could you point me to your test dir for your experiment of "Build FV3ATM+HYCOM+CMEPS / Run FV3ATM+HYCOM"? My test with this configuration did not work properly (the SST field in FV3ATM did not come from HYCOM as expected). My test of "Build FV3ATM+HYCOM+CMEPS / Run FV3ATM+HYCOM+CMEPS", however, worked properly for the ocean coupling.

I also looked at @BijuThomas-NOAA 's two tests with cpl_ocean=3 (CMEPS based coupling) and cpl_ocean=2 (direct coupling) under these two dirs:
/work/noaa/hwrf/scrub/bthomas/hafs_couplehycom_cplocean3/2019082900/00L/forecast
/work/noaa/hwrf/scrub/bthomas/hafs_couplehycom_cplocean2/2019082900/00L/forecast
Again, the cpl_ocean=3 experiment worked fine, whereas in the cpl_ocean=2 experiment, the FV3ATM side SST did not come from the HYCOM side. I was wondering if the nems.configure file used for the cpl_ocean=2 option was set properly.

Bin

@danrosen25
Copy link
Copy Markdown

@BinLiu-NOAA
I found the bug in HYCOM. The call to set_impexp_fields should be outside the if statement. Otherwise the default set of coupled fields is not initialized. This doesn't affect the CMEPS coupled run because it's not configured to use the defaults. I'm fixing now.

hafs-community/HYCOM-src@7a0f867#diff-2c8808beb54881c9156f003a985721ce97852fdfe09fec654f0661c64eb0547eR302

…e direct

coupling when the forecast model is built with the CMEPS=Y option.
Comment thread parm/forecast/regional/nems.configure.atm_ocn.tmp
feature/hafs_couplehycom branch, which the HYCOM set_impexp_fields fix.
@danrosen25
Copy link
Copy Markdown

@BinLiu-NOAA
I made the fix here: HYCOM-src
And updated the ufs-weather-model here with a PR back to support/HAFS

And I updated the ufs-weather-model revision for this PR

@danrosen25
Copy link
Copy Markdown

nems.configure.atm_ocn.tmp

The changes to this file will work without CMEPS in the build but I fixed the issue anyway.

Comment thread parm/forecast/regional/nems.configure.atm_ocn.tmp Outdated
@hyunsookkim-NOAA
Copy link
Copy Markdown

@BinLiu-NOAA, then do you get HYCOM imports forcing variables from FV3 successfully?

* Update nems.configure.atm_ocn.tmp for the direct coupling configuration
@BinLiu-NOAA
Copy link
Copy Markdown
Collaborator Author

@BinLiu-NOAA, then do you get HYCOM imports forcing variables from FV3 successfully?

@hyunsookkim-NOAA, yes, with @danrosen25 's fix, we now have the HAFS FV3ATM-HYCOM coupling working properly for different coupling configurations, including for both direct coupling and CMEPS based coupling.

@BinLiu-NOAA
Copy link
Copy Markdown
Collaborator Author

I should also mention that I'm testing 'develop' merged locally with 'feature/hafs_couplehycom' and this may be an issue from the 'develop' branch side.

@danrosen25 This feature/hafs_couplehycom branch has already been synced with the latest HAFS develop. And once this PR goes in, it will become the develop. So, no need to locally merge develop backward.

Copy link
Copy Markdown

@uturuncoglu uturuncoglu left a comment

Choose a reason for hiding this comment

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

I can see that sorc/hafs_forecast.fd also bring some CDEPS related additions such as model level RT. Please note that RT test does not work with the updated version of the model yet. I'll look at it and fix it under feature/hafs_couplehycom_cdeps branch for the model.

Comment thread parm/hafs.conf
Comment thread sorc/build_forecast.sh
@BinLiu-NOAA
Copy link
Copy Markdown
Collaborator Author

I can see that sorc/hafs_forecast.fd also bring some CDEPS related additions such as model level RT. Please note that RT test does not work with the updated version of the model yet. I'll look at it and fix it under feature/hafs_couplehycom_cdeps branch for the model.

@uturuncoglu Thanks for the note! And yes, we so far only test the standard regression tests in the ufs-weather-model level. We understand that the CDEPS related regression tests are not working. But, as you mentioned, you are working on fixing them in the feature/hafs_couplehycom_cdeps branch.

@danrosen25
Copy link
Copy Markdown

@BinLiu-NOAA
I tried a fresh checkout of 5b3a8b1 and it still failed building build_vortextracker.sh. (same error)
/scratch1/NCEPDEV/hwrf/save/Daniel.Rosen/hafs_couplehycom_dev/sorc/logs

Hi @BinLiu-NOAA,
The issue was related to my login environment. Once I turned off the modules I was loading during login the vortex build works. Should I log this as an issue here: https://github.com/NOAA-EMC/HAFS/issues. It might be as simple as changing the top line in the build_vortextracker.sh script.

@hafs-community hafs-community deleted a comment from BinLiu-NOAA Feb 1, 2021
@hafs-community hafs-community deleted a comment from BinLiu-NOAA Feb 1, 2021
@BinLiu-NOAA
Copy link
Copy Markdown
Collaborator Author

BinLiu-NOAA commented Feb 4, 2021

I have just synced the hafs_forecast.fd with its latest develop branch as of 02/04/2021 (no HAFS result changes). Also, this branch has synced with the latest HAFS develop branch. Appreciate it if someone can review and approve this pull request, so that we can move on to our next pull request. Thanks!

@evankalina
Copy link
Copy Markdown
Contributor

All, the code changes look reasonable, but I would like to run the regression tests on Jet before approving. Is that a reasonable way to test these changes? Basically I'm looking for input on how to be an effective reviewer on these PRs ;-)

@BinLiu-NOAA
Copy link
Copy Markdown
Collaborator Author

BinLiu-NOAA commented Feb 5, 2021

All, the code changes look reasonable, but I would like to run the regression tests on Jet before approving. Is that a reasonable way to test these changes? Basically I'm looking for input on how to be an effective reviewer on these PRs ;-)

Hi @evankalina, for this one you probably do not need to run the regression test. I was thinking of running all the regression tests on all supported platforms for our upcoming pull request to merge the feature/hafsv0.2_baseline branch (which has already synced up with this feautre/hafs_couplehycom branch) into the develop. I will initiate that pull request right after this one goes into develop.

@evankalina
Copy link
Copy Markdown
Contributor

@BinLiu-NOAA OK, sounds good. I am OK with this going to develop.

@BinLiu-NOAA
Copy link
Copy Markdown
Collaborator Author

Thanks, @evankalina and @ChunxiZhang-NOAA!
Merging this pull request in now.

@BinLiu-NOAA BinLiu-NOAA merged commit 2c55fd9 into develop Feb 5, 2021
@danrosen25
Copy link
Copy Markdown

danrosen25 commented Feb 5, 2021

Thanks @BinLiu-NOAA @evankalina
I removed 'feature/hafs_couplehycom' branches now that they are merged.
From:

@danrosen25 danrosen25 deleted the feature/hafs_couplehycom branch February 5, 2021 22:02
mrinalbiswas pushed a commit that referenced this pull request Aug 24, 2021
Add the CMEPS based HAFS-HYCOM coupling into the HAFS workflow

Bring in the CMEPS based HAFS-HYCOM coupling by merging the feature/hafs_couplehycom branch into the develop branch
Update hafs_forecast.fd to point to the latest support/HAFS branch, which now has the CMEPS based HYCOM coupling capability developed by @danrosen25, et al.
Update the HAFS workflow to support the CMEPS-based HYCOM coupling. The cpl_ocean configuration option is to control the different coupling methods in the workflow.
- 0, run side-by-side forecast without coupling
- 1, direct NUOPC/ESMF coupling through nearest point regridding
- 2, direct NUOPC/ESMF coupling through bilinear regridding
- 3, CMEPS based coupling through bilinear regridding
Add a new CCPP suite definition file: suite_HAFS_v0_gfdlmp_nonsst.xml
Note: The cpl_ocean option 3 currently gets slightly different results from the cpl_ocean option 2, which is caused by the term ordering differences in CMEPS resulting in small roundoff differences that build each coupling time step. But, @danrosen25 was able to reproduce bit-for-bit results if using a different branch of ESMF and turning on some regridding options that affect term ordering. Overall, it's just the butterfly effect, with about a 10^-14 difference during each regridding call.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants