Update MOM6 cap for 2-way ocean-wave coupling for HAFS#143
Update MOM6 cap for 2-way ocean-wave coupling for HAFS#143jiandewang merged 14 commits intoNOAA-EMC:dev/emcfrom
Conversation
|
I think doing this via a hard-coded FillValue is problematic. It may work in UFS, but what about other users of the cap? Have you tried implementing this via a logical control flag (eg You're also only checking one component of the stokes drift. |
|
agree with @DeniseWorthen , NCAR is also using nuopc driver for their system |
|
@DeniseWorthen ,@jiandewang Thank you for the suggestions. I will try to use the case_name option. |
|
@binli2337 I am trying to understand your logic here. |
|
@jiandewang For non-hafs cases, the case_name might be ufs.cpld, ufs.atmw, DATM_CFSR, DATM_GEFS_NEW, etc. You can find the setting for case_name in the parm/ufs.configure*IN files. |
| real(ESMF_KIND_R8), allocatable :: stkx(:,:,:) | ||
| real(ESMF_KIND_R8), allocatable :: stky(:,:,:) | ||
| character(len=*) , parameter :: subname = '(mom_import)' | ||
| real(ESMF_KIND_R8), parameter :: fillValue = 9.99e20_ESMF_KIND_R8 |
There was a problem hiding this comment.
@sanAkel The missing value (or fillvalue) is set in CMEPS and imported to MOM6 cap and WW3 cap. Updates in this PR are to provide a temporary fix by converting the missing value to 0 so that the coupled HAFS application can run for the moment. Once WW3 output from a global model is available, the global WW3 output can provide valid values in the non-overlapped regions and this fix can be removed. The updates in this PR are made in the cap so that these updates do not affect other applications using the MOM6 model. A namelist option (for the zero number) is not used because we plan to convert the missing value to 0 (the only option). Thanks for reviewing the updates.
There was a problem hiding this comment.
@binli2337 Sorry---we're committing this to our repos, only to then turn around and remove it? Why does it need to be committed outside of your own forks in hafs-community?
There was a problem hiding this comment.
@DeniseWorthen Before a global coupled forecast system is in operation, we need these updates for a possible operational HAFS system that includes two-way ocean-wave coupling. It is better to use the updated model code from the develop branch of ufs-weather-model. Thanks!
There was a problem hiding this comment.
@binli2337 Regardless of the order/sequence of merge,
my suggestion is to remove parameter in:
real(ESMF_KIND_R8), parameter :: fillValue = 9.99e20_ESMF_KIND_R8
There was a problem hiding this comment.
You can always update your HAFS community from UWM-develop and have 'updated model code'. What goes operational---the code from HAFS community or the code from UWM-develop?
There was a problem hiding this comment.
@DeniseWorthen The ocean domain and wave domain in the hafs tests are basically overlapped. (98W-8W,1.5N-45.5N) for WW3 and (-98-8W, 1.76N-45.8N) for MOM6. I will add a new parameter in the ufs.configure to indicate that the missing stokes drift is set to zero or not. The default setting is false. For the HAFS with MOM6 applications, the parameter will be set to true if the ocean and wave domain are nearly overlapped. Thanks for reviewing the code.
There was a problem hiding this comment.
@sanAkel The missing value (or fillvalue) is set in CMEPS and imported to MOM6 cap and WW3 cap. Updates in this PR are to provide a temporary fix by converting the missing value to 0 so that the coupled HAFS application can run for the moment. Once WW3 output from a global model is available, the global WW3 output can provide valid values in the non-overlapped regions and this fix can be removed. The updates in this PR are made in the cap so that these updates do not affect other applications using the MOM6 model. A namelist option (for the zero number) is not used because we plan to convert the missing value to 0 (the only option). Thanks for reviewing the updates.
@binli2337 this will put dev/emc MOM6 repo. in a very bad status as I won't be able to ask MOM6 community to review and push it to its main repository. In the meantime if EMC has another internal PR, then it has to be build upon this PR which will again prevent me from asking external to review and push back to main (before you revert your PR).
|
@jiandewang We can coordinate on when to send PRs to be merged with mom-ocean. But meanwhile, I think we should let the HAFS group's work proceed and progress ... when they have resolved ⬆️. |
There was a problem hiding this comment.
Has this feature/addition been tested? If yes, @binli2337 can you please include a summary of results or STDOUT to back up the claim that it all works, at least mechanically?
|
@sanAkel All the regression tests in the UFS were run with this updated MOM6. The test log is at https://github.com/binli2337/ufs-weather-model/blob/202501b/ocean_wave_update/tests/logs/RegressionTests_hera.log. There are 268 tests and 263 passed and 5 hafs tests with ww3 component changed results due to a new model definition file used for those 5 tests. |
|
Excellent! Can you please copy/paste relevant lines of output- figure or something showing that things are working? |
|
@sanAkel A new feature branch of MOM6 was created to check whether the updated code to set missing Stokes drift components to zero is executed correctly. The new branch is at https://github.com/binli2337/MOM6/tree/feature/check_ocean_wave_update. When this new branch is used in a regression test, parts of the MOM6 cap output file "PET360.ESMF_LogFile" are shown below: Without the code to set missing Stokes drift to zero, the test will fail because the missing values (9.99e20) are used in the calculations. Complete test results are located at /scratch1/NCEPDEV/stmp2/Bin.Li/FV3_RT/rt_3955108/hafs_regional_storm_following_1nest_atm_ocn_wav_mom6_intel. Thanks! |
|
Output (logs) reflects the expected outcome. |
|
associated UFW PR: ufs-community/ufs-weather-model#2584 |
|
@binli2337 can you change it to Gulf of American at https://github.com/binli2337/MOM6/blob/feature/ocean_wave_update/src/tracer/oil_tracer.F90#L497 I scanned the whole package of MOM6 code, this is the only place that we are using "Mexico" |
|
comments from NCAR @alperaltuntas: I ran a test case and confirmed that it doesn't appear to impact our configuration. However, I am wondering whether these changes can be made more general. I see that Santha has already suggested turning the hardcoded 9.99e20 to a configuration variable, which I agree with. Also, we might also want to turn on this feature within CESM, so I think the following if statement is too restrictive: if( (trim(casename) == "ufs.hafs") ping @binli2337 |
Following comments from @alperaltuntas- which should be addressed.
|
@jiandewang The missing value (9.99e20) is defined in CMEPS by other developers and other components need to use the same value to check whether fields transferred from CMEPS are missing or not. I agree that the statement [if( (trim(casename) == "ufs.hafs") ] is too restrictive and I will remove it. Thanks! |
|
@binli2337 Thanks for expediting:
Reg the following, we can circle back with @alperaltuntas and other CMEPS co-developers sometime later:
(I do see other instances of set values that you allude to; perhaps a clean up will have to be coordinated in a future PR when time/resources permit.) @alperaltuntas are ok with the ⬆️ ? |
|
@sanAkel The code you point to for "huge" is used only when CMEPS is configured to call into ccpp as a host model. |
@DeniseWorthen it was a randomly picked example to acknowledge @binli2337's ⬆️ point. I'm sure you/others can find better ones! |
|
@jiandewang @sanAkel we are trying to run a full test at ufs-community/ufs-weather-model#2584. Please, go ahead for review and approve. I will make a note to merge once all the tests complete w/o issue. |
|
@binli2337 can you sync MOM6 to latest commit |
|
@binli2337 I will do squash merging, could you write a sentence on what should be included in my final commit history ? |
|
@jiandewang The MOM6 cap is updated to convert a missing value to zero for the imported Stokes drift components. |
|
|
All tests are done ok at ufs-community/ufs-weather-model#2584. @sanAkel @jiandewang can you merge this pr? |
The mom_cap_methods.F90 file located in the MOM6/config_src/drivers/nuopc_cap directory is revised to convert imported Stokes drift components from a missing value (9.99e20) to zero. This update doesn't have any impacts on global applications.
These updates are only needed when the ocean model domain is greater than the wave model domain and the wave model cannot provide Stokes drift components for the entire ocean model domain.