Bring CMEPS mediator compatibility to WW3 cap

[uturuncoglu]

Pull Request Summary

This PR aims to bring capability to run WW3 wave component through the use of CMEPS mediator. The initial implementation is done as a part of HAFS application and supports ATM-WAV and ATM-OCN-WAV (with HYCOM) configurations.

Description

This PR adds following,

• The corner coordinates is added to WW3 grid in the ESMF/NUOPC cap. This is required for CMEPS since ESMF Mesh is used to represent model grids/meshes and ESMF Mesh generation routine requires corner coordinates. This is also required for conservative regridding.
• mediator_present = .true. needs to be added to WAV_attributes to use CMEPS mediator.
• Since WW3 and FV3 domains does not overlap entirely in the regional HAFS application. The data coming from CMEPS is merged with the data read from wind forcing file (wind.ww3). The special subroutine in the cap reads the forcing file and merge with the data coming from coupling to provide complete set of wind field to WW3. The merge mask is also generated on the fly by checking fill value of the fields coming from CMEPS mediator. This is also controlled by extra attribute merge_import = .true. in WAV_attributes and it is not required for the global application.
• The full regression test suit in the UFS Weather model does not indicate any answer change and existing WW3 coupled configurations that uses NUOPC connector rather than CMEPS runs without any issue and answer changes.

Issue(s) addressed

• Is there an issue associated with this development (bug fix, enhancement, new feature)?
  WW3 can be used with CMEPS mediator to exchange data among the components.

Check list

• Is your feature branch up to date with the authoritative repository (NOAA/develop)? Yes
• Make sure you have checked the checklist for a developer submitting to develop, checklist for a developer submitting to develop and updating version number
• Reviewers: @JessicaMeixner-NOAA

Testing

• How were these changes tested?
  Full UFS Weather Model RTs were run including existing configurations with wave coupling
• Are the changes covered by regression tests? (If not, why? Do new tests need to be added?)
  There is no change in the regression tests
• If a new feature was added, was a new regression test added?
  N/A
• Have regression tests been run?
  Yes, full regression tests were run without any issue on Orion and Hera.
• Which compiler / HPC you used to run the regression tests in the PR?
• Please provide the summary output of matrix.comp (matrix.Diff.out, matrixCompFull.out and matrixCompSummary.out):
  Please indicate the expected changes in the outputs (excluding the known list of non-identical tests).

[JessicaMeixner-NOAA]

@uturuncoglu can you update the manual (this is the relevant part: https://github.com/NOAA-EMC/WW3/blob/develop/manual/app/nuopc.tex) or at least create a new issue so that we can track that these updates need to be added to the manual at some point?

[uturuncoglu]

@JessicaMeixner-NOAA I think that creating issue about the documentation is better idea since we did not finalize the code and it could change it slightly based on the test of the global application but if you want me to add section to the documentation that is also fine for me.

[uturuncoglu]

@JessicaMeixner-NOAA I created an issue about the documentation. Here is the link #399

[aliabdolali]

I ran the full regtest on hera with expected known non-b4b tests

mww3_test_03
mww3_test_07
ww3_tp2.10
ww3_tp2.15
ww3_tp2.18

CMEPS.zip

[JessicaMeixner-NOAA]

The regression tests that will matter most for this is to test the ufs-weather-model. However, we should probably wait for the WW3 PR that updates the code before running those.

@danrosen25 @BinLiu-NOAA anyone else want to review this code? I have done my first quick review and regression tests for previous ufs-weather-model passed, but they should be reviewed and I should probably do a more in-depth review as well.

[aliabdolali]

@JessicaMeixner-NOAA I think we should associate a milestone to this development on github and define the version accordingly.

[JessicaMeixner-NOAA]

@aliabdolali sounds good.

@danrosen25 @BinLiu-NOAA @uturuncoglu @aliabdolali I'm going to set up a meeting to do a group review of this code.

[aliabdolali]

Here is the milestone
https://github.com/NOAA-EMC/WW3/milestone/25

[danrosen25]

This by-passes the import data-dependency. The import data-dependency restarts winds from an external component. During CMEPS restart runs this DataInitialize subroutine should check imported data time stamp to determine if CMEPS has restarted winds.

[uturuncoglu]

Yes, we need to revisit this part once we have a configuration that restarts the model and uses CMEPS for coupling. I think there is no need to take action at this point.

[JessicaMeixner-NOAA]

I think this could be a fairly major issue here. If it's not addressed now, there needs to be an issue documenting that this is happening and that we need to look into it further.

[uturuncoglu]

@JessicaMeixner-NOAA new issue - #428 - is created to track it.

[JessicaMeixner-NOAA]

Check to make sure we can't get to this section of code w a unstructured grid and we need to have a if GTYPE if statement

[JessicaMeixner-NOAA]

I checked and we only get to this routine for the select GTYPES, so no need for a if GTYPE statement:

    if ( (GTYPE.eq.RLGTYPE).or.(GTYPE.eq.CLGTYPE) ) then
        write(msg,'(a)') trim(cname)// &
          ': Creating import grid for Reg/Curvilinear Mode'
        call ESMF_LogWrite(trim(msg), ESMF_LOGMSG_INFO)
        call CreateImpGrid( gcomp, rc=rc )
        if (ESMF_LogFoundError(rc, PASSTHRU)) return
     elseif (GTYPE.eq.UNGTYPE) then
        write(msg,'(a)') trim(cname)// &
          ': Creating import mesh for Unstructured Mode'
        call ESMF_LogWrite(trim(msg), ESMF_LOGMSG_INFO)
        call CreateImpMesh( gcomp, rc=rc )

[uturuncoglu]

Is this related with the adding corner coordinates? I am asking because there is no such check in the code that adds corner coordinates.

[JessicaMeixner-NOAA]

@uturuncoglu thanks for being patient with me as I understand all of this better. I was able to access your directory and will look at the results more closely today which I think will help me see what you have commented here.

[uturuncoglu]

@JessicaMeixner-NOAA Sure. Let me know if you find anything that needs to be fixed.

[JessicaMeixner-NOAA]

I made one more comment noting an area where we can improve on efficiency if needed in the future. Since both WW3 and ufs-weather-model have updated since the last time the ufs-weather-model regression tests were run using this branch, we should probably do that one more time before merging. Otherwise, I think this is good to go. Thanks Ufuk and sorry for the delay in my review.

[JessicaMeixner-NOAA]

@uturuncoglu I ran the ufs-weather-model regtests w/your branch (after merging in WW3 develop) and the regression tests did not pass. I didn't get a chance to figure out why, but wanted to let you and @aliabdolali know so we can look into that before merging this PR.

[uturuncoglu]

@JessicaMeixner-NOAA as remember correctly, it was passing before right? I also remember that I run full RTs and it was passing. We have following possibilities:

• @BinLiu-NOAA synced the branch and it could be an issue over there.
• It could be related to the recent changes that I did after your PR comments. We could still go back and test the old version to find the commit that causes issue.

Let me look at those and I'll update you about it.

[uturuncoglu]

Sorry, I closed and reopen the PR. It is my fault.

[uturuncoglu]

@JessicaMeixner-NOAA I run the cpld_control_wave and it fails with following trace that goes to WW3 cap.

/work/noaa/gmtb/tufuk/HAFS_wav/sorc/hafs_forecast.fd/WW3/model/tmp/wmesmfmd.F90:1384
/work/noaa/gmtb/tufuk/HAFS_wav/sorc/hafs_forecast.fd/WW3/model/tmp/wmesmfmd.F90:3109

This is the line of code that perform halo update,

!
! 3.b Store import field halo routeHandle
!
      call ESMF_FieldHaloStore( impMask, routeHandle=impHaloRH, rc=rc )
      if (ESMF_LogFoundError(rc, PASSTHRU)) return

So, it does not makes sense to me. Anyway, I'll try to roll back my recent changes to see what happens. The only think that is in my mind is the commit that removes control to add corner coordinates to import and export grid. Anyway, i'll update you about it.

[uturuncoglu]

I go back just before commit that removes the check for adding corner coordinates and model seems working and this particular test passes. So, it seems that hafs-community@f492972 commit has the issue if we run it with connectors. At this point I am not sure why it is failing. Maybe halo region is not defined correctly in corner coordinates or maybe it is an issue in ESMF side. I'll check and let you know.

[uturuncoglu]

@JessicaMeixner-NOAA I think I solved the issue. Please see following commit 1f1ed07. I also run cpld_control_wave and it passes now. I could also run other wave configurations but I am not expecting any issue with others. I'll also check the HAFS wave configurations too and let you know. BTW, the debug section in CreateImpGrid and CreateExpGrid just creates tmp field in center stagger to write out grid coordinates. It might be useful to do the same for corner stagger. We could create another subroutine similar to this call used in CMEPS to write all the grid coordinates, mask etc. to a single file. This seems more elegant way rather than writing them into individual files. Let me know what do you think?

[uturuncoglu]

@JessicaMeixner-NOAA I run all wave related regression tests (cpld_bmark_wave, cpld_bmarkfrac_wave, cpld_bmarkfrac_wave_v16 and cpld_control_wave) and all of them were passed on Orion. I also tested fully coupled FV3+WW3 regional configuration (two-way interaction) under HAFS applications and it is also working without any issue.

[JessicaMeixner-NOAA]

@uturuncoglu thanks for tracking down and solving the issue. I'll retest on hera today to confirm your results on orion.

[JessicaMeixner-NOAA]

After hera maintenance, I ran the full ufs-weather-model regression tests with hera.intel with this branch of WW3 + merging in the develop branch and one test failed:
Test control_atm_aerosols 093 failed in run_test failed
@uturuncoglu you you just ran the wave tests on orion, correct? I feel fairly confident that atm->aerosols was not effected by this change in WW3. I'm going to re-run just that test to see if it was a one-off error.

[JessicaMeixner-NOAA]

re-running the failed control_atm_aerosols test, it passed this time w/hera.intel

[JessicaMeixner-NOAA]

A group review of this code was completed and outstanding items were either addressed or issues for follow up were made.

The ufs-weather-model (using commit hash b64e648c34bd177a761195fe0bf17da6be44fc09) + this branch with the WW3 develop merged in passed all regression tests for me on hera.intel

Thanks @uturuncoglu for all of the detailed explanations and these updates!!!!

[uturuncoglu]

@JessicaMeixner-NOAA that is great news. I think that PR is in better shape now with your comments and suggestions.

[aliabdolali]

The regtest passed successfully on hera using intel,impi, with expected known non-b4b cases:

mww3_test_03/./work_PR2_UQ_MPI_d2                     (9 files differ)
mww3_test_03/./work_PR2_UNO_MPI_d2                     (8 files differ)
mww3_test_03/./work_PR3_UQ_MPI_d2_c                     (8 files differ)
mww3_test_03/./work_PR3_UQ_MPI_d2                     (8 files differ)
mww3_test_03/./work_PR3_UNO_MPI_d2_c                     (8 files differ)
mww3_test_03/./work_PR1_MPI_d2                     (8 files differ)
mww3_test_03/./work_PR3_UNO_MPI_d2                     (6 files differ)
mww3_test_07/./work_PR3_UQ                     (3 files differ)
ww3_tp2.10/./work_MPI_OMPH                     (6 files differ)
ww3_tp2.15/./work_5km                     (1 files differ)
ww3_tp2.15/./work_MPI_5km                     (1 files differ)

matrixCompFull.txt
matrixCompSummary.txt
matrixDiff.txt

[aliabdolali]

@ danrosen25 could you provide your review, so we can move on and merge this PR.

[uturuncoglu]

@danrosen25 do you have anything that needs to be addressed?

[danrosen25]

I haven't been able to run regression tests but I'm holding up the completion of this PR. I am not keen on the use of med_present as this creates an end-user configuration that needs documentation for a package of features that seem to be independent on med_present=t/f. There are still a few things that will need to be addressed such as data dependencies during initialization

[JessicaMeixner-NOAA]

@danrosen25 @uturuncoglu are we in such a rush that we need to merge things we aren't okay with? At minimum can we make an issue to address the "if med_present" issue? What are the alternatives for getting rid of this if- mediator statements?

We currently have an issue for the initialization:
#428 and documentation #399 (@uturuncoglu should we go ahead an assign those to you?)

With not running the regression tests, was this a time issue or a the regression tests aren't working issue?

[uturuncoglu]

@JessicaMeixner-NOAA I think that we already remove most of med_present such as from adding corner coordinates etc. I think removing all of them will take time since I have no too much time under HAFS project. I need to wrap-up everything including CDEPS, WW3 work until end of this month. Of curse you could assign those issues to me but I am not sure when I will find time to work on those because I am supposed to work on UFS-xgrid implementation at the beginning of August. I am not sure about you questions related to the RTs. As I know the wave related RTs are passing without any issue.

[uturuncoglu]

@JessicaMeixner-NOAA currently med_present only used for following,

• adding scalar field to export state. this is CMEPS specific and required to have 2d fields in mediator history and restart.
• not checking field time stamps in Data initialization, which is related with the WW3 ESMF/NUOPC cap fails in DataInitialize() phase if NUOPC_IsAtTime() is used with CMEPs coupling #428 issue.

So, we don't have too much related with med_present at this point.

[danrosen25]

@JessicaMeixner-NOAA
There will be a progress report next week for hurricane supplemental progress and if we can close this before the report that would be better.

[JessicaMeixner-NOAA]

Sounds good.. Note that there needs to be a separate ufs-weather-model PR for this or you can work with @aliabdolali as he has an open ufs-weather-model PR that involves updating WW3 to the top of develop that is not yet merged