Skip to content

Update MOM6 cap for 2-way ocean-wave coupling for HAFS#143

Merged
jiandewang merged 14 commits intoNOAA-EMC:dev/emcfrom
binli2337:feature/ocean_wave_update
Aug 11, 2025
Merged

Update MOM6 cap for 2-way ocean-wave coupling for HAFS#143
jiandewang merged 14 commits intoNOAA-EMC:dev/emcfrom
binli2337:feature/ocean_wave_update

Conversation

@binli2337
Copy link
Copy Markdown

@binli2337 binli2337 commented Jan 19, 2025

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.

@DeniseWorthen
Copy link
Copy Markdown
Collaborator

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 case_name) which for the hafs application is ufs.hafs.

You're also only checking one component of the stokes drift.

@jiandewang
Copy link
Copy Markdown
Collaborator

agree with @DeniseWorthen , NCAR is also using nuopc driver for their system

@binli2337
Copy link
Copy Markdown
Author

@DeniseWorthen ,@jiandewang Thank you for the suggestions. I will try to use the case_name option.

@jiandewang
Copy link
Copy Markdown
Collaborator

@binli2337 I am trying to understand your logic here.
in mom-cap you read in "case_name", then you added case_name in mom_inport subroutine agument list.
what value it will be for "case_name" in cap code when you call mom_import for non-hafs case ? It has to be assigned a value as it isn't optional in subroutine.

@binli2337
Copy link
Copy Markdown
Author

@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
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Please see this suggestion

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

@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.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

@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?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

@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!

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

@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

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

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?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

@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.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

@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).

@sanAkel
Copy link
Copy Markdown
Collaborator

sanAkel commented Feb 12, 2025

@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 ⬆️.

Copy link
Copy Markdown
Collaborator

@sanAkel sanAkel left a comment

Choose a reason for hiding this comment

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

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?

@binli2337
Copy link
Copy Markdown
Author

@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.

@sanAkel
Copy link
Copy Markdown
Collaborator

sanAkel commented May 29, 2025

Excellent!

Can you please copy/paste relevant lines of output- figure or something showing that things are working?

@binli2337
Copy link
Copy Markdown
Author

@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:

20250529 180531.593 INFO             PET360 (MOM_cap:ModelAdvance)------>Advancing OCN from: 2020  8 25 12  0  0   0
20250529 180531.593 INFO             PET360 --------------------------------> to: 2020  8 25 12  6  0   0
20250529 180531.594 INFO             PET360 set ice_ocean_boundary%ustkb=0 here       1       1       3
20250529 180531.594 INFO             PET360 set ice_ocean_boundary%ustkb=0 here       2       1       3
20250529 180531.594 INFO             PET360 set ice_ocean_boundary%ustkb=0 here       3       1       3
20250529 180531.594 INFO             PET360 set ice_ocean_boundary%ustkb=0 here       4       1       3
20250529 180531.594 INFO             PET360 set ice_ocean_boundary%ustkb=0 here       5       1       3
20250529 180531.594 INFO             PET360 set ice_ocean_boundary%ustkb=0 here       6       1       3
20250529 180531.594 INFO             PET360 set ice_ocean_boundary%ustkb=0 here       7       1       3
20250529 180531.594 INFO             PET360 set ice_ocean_boundary%ustkb=0 here       8       1       3
20250529 180531.594 INFO             PET360 set ice_ocean_boundary%ustkb=0 here       9       1       3

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!


@sanAkel sanAkel self-requested a review May 30, 2025 22:20
sanAkel
sanAkel previously approved these changes May 30, 2025
@sanAkel
Copy link
Copy Markdown
Collaborator

sanAkel commented May 30, 2025

Output (logs) reflects the expected outcome.

@jiandewang
Copy link
Copy Markdown
Collaborator

associated UFW PR: ufs-community/ufs-weather-model#2584

@jiandewang
Copy link
Copy Markdown
Collaborator

@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"

@jiandewang
Copy link
Copy Markdown
Collaborator

jiandewang commented Jul 4, 2025

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

@sanAkel sanAkel dismissed their stale review July 12, 2025 05:07

Following comments from @alperaltuntas- which should be addressed.

@binli2337
Copy link
Copy Markdown
Author

@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!

@sanAkel
Copy link
Copy Markdown
Collaborator

sanAkel commented Jul 14, 2025

@binli2337 Thanks for expediting:

I agree that the statement [if( (trim(casename) == "ufs.hafs") ] is too restrictive and I will remove it.

Reg the following, we can circle back with @alperaltuntas and other CMEPS co-developers sometime later:

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 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 ⬆️ ?

@DeniseWorthen
Copy link
Copy Markdown
Collaborator

@sanAkel The code you point to for "huge" is used only when CMEPS is configured to call into ccpp as a host model.

@sanAkel
Copy link
Copy Markdown
Collaborator

sanAkel commented Jul 14, 2025

@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!

@jkbk2004
Copy link
Copy Markdown

jkbk2004 commented Aug 6, 2025

@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.

@jiandewang
Copy link
Copy Markdown
Collaborator

@binli2337 can you sync MOM6 to latest commit

@jiandewang
Copy link
Copy Markdown
Collaborator

@binli2337 I will do squash merging, could you write a sentence on what should be included in my final commit history ?

@binli2337
Copy link
Copy Markdown
Author

@jiandewang The MOM6 cap is updated to convert a missing value to zero for the imported Stokes drift components.

@sanAkel
Copy link
Copy Markdown
Collaborator

sanAkel commented Aug 9, 2025

@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.

@jkbk2004

@jkbk2004
Copy link
Copy Markdown

All tests are done ok at ufs-community/ufs-weather-model#2584. @sanAkel @jiandewang can you merge this pr?

@jiandewang jiandewang merged commit 3970dbd into NOAA-EMC:dev/emc Aug 11, 2025
52 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants