Pass new `seaice_melt` field from SIS2 to MOM6 #710

hdrake · 2024-08-15T17:20:31Z

See companion PRs NOAA-GFDL/SIS2#214 and NOAA-GFDL/FMScoupler#145

See companion commit hdrake/SIS2@1c8dc7e

Some other minor bug fixes

config_src/drivers/FMS_cap/MOM_surface_forcing_gfdl.F90

theresa-cordero · 2024-08-19T20:10:49Z

Were you able to test the changes to the non-FMS caps with the associated couplers?

theresa-cordero · 2024-08-19T20:19:25Z

Is the expected behavior here that lprec_new + seaice_melt = lprec_old? And if so, do you find that to be the case?
lprec_new being the the liquid precititation into the ocean after these changes and lprec_old being the liquid precititation before these changes.

hdrake · 2024-08-19T21:08:37Z

Is the expected behavior here that lprec_new + seaice_melt = lprec_old? And if so, do you find that to be the case?
lprec_new being the the liquid precititation into the ocean after these changes and lprec_old being the liquid precititation before these changes.

Yes, that is the expected behavior. I have not done a careful side by side comparison to verify they are the same, but visually they look like it.

What kind of workflow do you use to directly compare two configurations? Checkout the different commits and manually run different configurations and compare the outputs?

hdrake · 2024-08-19T21:09:29Z

Were you able to test the changes to the non-FMS caps with the associated couplers?

No, I've only tested with the Baltic_OM4_05 configuration that I believe uses the FMS cap.

theresa-cordero · 2024-08-19T21:13:54Z

What kind of workflow do you use to directly compare two configurations? Checkout the different commits and manually run different configurations and compare the outputs?

Yes. Checkout and compile the dev/gfdl code, run and save lprec. Then recompile with all changes are rerun the experiment saving lprec and seaice_melt. I think it would be sufficient to test the final versions of MOM6, SIS2, and the coupler and not every commmit along the way (assuming of course that it works).

hdrake · 2024-08-19T22:49:35Z

Were you able to test the changes to the non-FMS caps with the associated couplers?

@theresa-morrison, are these particular experiments I should test with? I guess all of the ones named SIS2_* in the MOM6-examples/ice_ocean_SIS2?

hdrake · 2024-08-20T00:18:28Z

Is the expected behavior here that lprec_new + seaice_melt = lprec_old? And if so, do you find that to be the case?
lprec_new being the the liquid precititation into the ocean after these changes and lprec_old being the liquid precititation before these changes.

Below is a comparison between the current MOM6-examples/dev/gfdl and my three PRs.

I do not have any idea why the two are different. I don't see how I could have gone wrong in separating the diagnostics in the source code...

hdrake · 2024-08-20T01:05:40Z

Is it possible that the answers with default parameter values have changed this much?

The boundary mass flux diagnostics seem internally consistent within each of the simulations:

(These residuals are about 1e-5 each of the individual terms in the budget, so indistinguishable from zero at single precision.)

theresa-cordero · 2024-08-20T14:24:37Z

There should be no parameter changes between the two experiments!
If you are using the version of MOM6 in MOM6 examples for the "old" experiment, you should instead use the version of dev/gfdl that this was branched from to make sure you have all the same defaults.

hdrake · 2024-08-20T14:35:19Z

If you are using the version of MOM6 in MOM6 examples for the "old" experiment, you should instead use the version of dev/gfdl that this was branched from to make sure you have all the same defaults.

Okay, yeah that makes sense. I'll figure out how to do that and regenerate these plots.

marshallward · 2024-08-20T14:43:49Z

Some additional comments

The changes to STALE_mct_cap and nuopc_cap should be removed. These caps are not meant to be used with the FMS coupler, and we don't maintain them. They already have their methods for handling ice-ocean boundaries.

Additionally, STALE_mct_cap is an archived copy of an unmaintained MCT cap, so it should never be changed.
Are modifications to MESO permitted? I am unsure if it's meant to be an archive of the original experiments, or if we should extend its support? (I suppose it should give identical answers either way? Maybe this is OK. I will confirm.)

Similar comments for BFB and dumbbell, although they are probably OK.

hdrake · 2024-08-20T16:35:37Z

There should be no parameter changes between the two experiments!
If you are using the version of MOM6 in MOM6 examples for the "old" experiment, you should instead use the version of dev/gfdl that this was branched from to make sure you have all the same defaults.

Okay, I did a clean comparison where the only changes were those in my three branches, and I confirmed there were no differences between the MOM_parameter_doc.all and SIS_parameter_doc.all files. However, the differences in the outputs shown above persisted. So it seems I must be doing something wrong in one of these PRs.

@marshallward

As requested by @marshallward NOAA-GFDL#710 (comment)

codecov · 2024-08-20T17:10:09Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 35.91%. Comparing base (9b45087) to head (13b9960).

Additional details and impacted files

@@             Coverage Diff              @@
##           dev/gfdl     #710      +/-   ##
============================================
- Coverage     37.03%   35.91%   -1.13%     
============================================
  Files           273      270       -3     
  Lines         82538    81496    -1042     
  Branches      15441    15427      -14     
============================================
- Hits          30568    29269    -1299     
- Misses        46253    46458     +205     
- Partials       5717     5769      +52

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

hdrake · 2024-12-11T22:46:59Z

This recent SIS2 commit (NOAA-GFDL/SIS2@5ecae71) to the related PR NOAA-GFDL/SIS2#214 fixes the bug discussed in the above #710 (comment). Essentially, I had removed the line of code that reset the calculation of the sea ice melt rate to zero every timestep,

net_melt(:,:) = 0.

and was just accumulating larger and larger melt rates every timestep...

After fixing this, this PR (and the related ones) now correctly separates the liquid precipitation and sea ice melt rate ocean diagnostics, as demonstrated by tests of the Baltic_OM4_05 configuration with and without the new code:

hdrake · 2024-12-11T22:47:28Z

@marshallward and @theresa-morrison, I think this is ready for review now!

config_src/drivers/FMS_cap/MOM_surface_forcing_gfdl.F90

src/core/MOM_forcing_type.F90

Hallberg-NOAA

This PR is incomplete. Because fluxes%seaice_melt was separated out from fluxes%lprec, they should always occur together. As such, I suspect that there need to be changes added to diagnostics/MOM_sum_output.F90, ice_shelf/MOM_marine_ice.F90 and ice_shelf/MOM_ice_shelf.F90, and there are still about 2 dozen places in MOM_forcing_types.F90 where fluxes%lprec is used but fluxes%seaice_melt is not. Please reexamine the indicated files for the missing code additions.

@Hallberg-NOAA

Resolves @Hallberg-NOAA's comment (https://github.com/NOAA-GFDL/MOM6/pull/710/files#r1938257295)

@Hallberg-NOAA

Addresses @Hallberg-NOAA's comment: https://github.com/NOAA-GFDL/MOM6/pull/710/files#r1938256849

breichl · 2025-03-14T16:21:43Z

config_src/drivers/FMS_cap/MOM_surface_forcing_gfdl.F90

@@ -604,7 +611,7 @@ subroutine convert_IOB_to_fluxes(IOB, fluxes, index_bounds, Time, valid_time, G,
    if (CS%use_net_FW_adjustment_sign_bug) sign_for_net_FW_bug = -1.
    do j=js,je ; do i=is,ie
      net_FW(i,j) = US%RZ_T_to_kg_m2s* &
-                    (((fluxes%lprec(i,j)   + fluxes%fprec(i,j)) + &
+                    ((((fluxes%lprec(i,j) + fluxes%seaice_melt(i,j)) + fluxes%fprec(i,j)) + &
                      (fluxes%lrunoff(i,j) + fluxes%frunoff(i,j))) + &
                      (fluxes%evap(i,j)    + fluxes%vprec(i,j)) ) * US%L_to_m**2*G%areaT(i,j)
      !   The following contribution appears to be calculating the volume flux of sea-ice


I think that seaice_melt might now be removed from the summation above, since the purpose of the code below appears to be to remove it from the net_FW flux.

We recently noticed some diagnosed imbalances in ice+ocean water conservation in OM5 with (CS%adjust_net_fresh_water_to_zero=.true.). If I understood the comments below, then I think this PR would take care of that non-conservation by no longer using the imperfect estimate for seaice_melt below.

Oh, I see. I will try to replacenet_FW with fluxes%seaice_melt directly then.

@Hallberg-NOAA

Continues to address @Hallberg-NOAA's [concern](NOAA-GFDL/SIS2#214 (comment)) that these diagnostics could change with the level of compiler optimization by adding parentheses to preserve the order in which fluxes are added together. However, there are some locations where I did not see how this was possible, as in this comment: NOAA-GFDL/SIS2#214 (comment)

Hallberg-NOAA · 2025-04-30T20:33:23Z

src/core/MOM_forcing_type.F90

@@ -2496,6 +2489,9 @@ subroutine get_net_mass_forcing(fluxes, G, US, net_mass_src)
  if (associated(fluxes%lprec)) then ; do j=js,je ; do i=is,ie
    net_mass_src(i,j) = net_mass_src(i,j) + fluxes%lprec(i,j)
  enddo ; enddo ; endif
+  if (associated(fluxes%seaice_melt)) then ; do j=js,je ; do i=is,ie


Moving up the contribution of fluxes%seaice_melt will change answers for people who are using the NUOPC coupler. Adding the contributions from fluxes%lprec and fluxes%seaice_melt separately will change answers for people using the FMS coupler. I suspect that we are going to need a runtime logical flag here as well to avoid changing somebody's answers.

Hallberg-NOAA · 2025-04-30T20:35:44Z

src/core/MOM_forcing_type.F90

      if (associated(fluxes%heat_content_lprec)) then
-        if (fluxes%lprec(i,j) > 0.0) then


I believe that these changes will change answers for people (like EMC and NCAR) that were already using fluxes%seaice_melt. We are probably going to need to control this with a runtime logical flag.

Hallberg-NOAA · 2025-04-30T20:38:09Z

src/core/MOM_forcing_type.F90

@@ -631,12 +634,9 @@ subroutine extractFluxes1d(G, GV, US, fluxes, optics, nsw, j, dt, &
    if (fluxes%evap(i,j) < 0.0) netMassOut(i) = netMassOut(i) + fluxes%evap(i,j)
  !   if (associated(fluxes%heat_content_cond)) fluxes%heat_content_cond(i,j) = 0.0 !??? --AJA

-    ! lprec < 0 means sea ice formation taking water from the ocean.
-    ! smg: we should split the ice melt/formation from the lprec
-    if (fluxes%lprec(i,j) < 0.0) netMassOut(i) = netMassOut(i) + fluxes%lprec(i,j)


The changes in the next ~8 lines will change answers for people who are using the NUOPC coupler and are already using fluxes%seaice_melt. A runtime flag might be needed here as well to recreate previous answers.

Hallberg-NOAA · 2025-04-30T20:40:27Z

src/core/MOM_forcing_type.F90

-                                        + fluxes%vprec(i,j)      )  &
-                                        + fluxes%seaice_melt(i,j))  &
-                                        + fluxes%frunoff(i,j)    ))
+                                         + fluxes%seaice_melt(i,j))  &


Both this reordering of the math and the one in about another 10 lines are needed to preserve answers with the FMS coupler, but will change answers with the NUOPC coupler. A runtime logical flag might be needed here to preserve both sets of answers, and make the default the setting that is needed with the NUOPC coupler, since active intervention will be needed to use seaice_melt with the FMS coupler.

Attempt to pass new seaice_melt field through the GFDL MOM6-SIS2 coupler

b3e65c6

See companion commit hdrake/SIS2@1c8dc7e

hdrake changed the title ~~Attempt to pass new seaice_melt field through the GFDL MOM6-SIS2 coupler~~ Attempt to pass new seaice_melt field through the GFDL MOM6-SIS2 coupler Aug 15, 2024

hdrake marked this pull request as draft August 15, 2024 17:27

Pass seaice_melt through all ice-ocean couplers

202ff90

Some other minor bug fixes

hdrake changed the title ~~Attempt to pass new seaice_melt field through the GFDL MOM6-SIS2 coupler~~ Attempt to new seaice_melt field from SIS2 to MOM6 Aug 16, 2024

hdrake marked this pull request as ready for review August 16, 2024 23:54

hdrake changed the title ~~Attempt to new seaice_melt field from SIS2 to MOM6~~ Pass new seaice_melt field from SIS2 to MOM6 Aug 16, 2024

theresa-cordero reviewed Aug 19, 2024

View reviewed changes

config_src/drivers/FMS_cap/MOM_surface_forcing_gfdl.F90 Outdated Show resolved Hide resolved

Check if seaice_melt is associated before the checksum

c8dc42e

theresa-cordero reviewed Aug 19, 2024

View reviewed changes

config_src/drivers/FMS_cap/MOM_surface_forcing_gfdl.F90 Show resolved Hide resolved

hdrake mentioned this pull request Aug 19, 2024

Pass seaice_melt field through ice-ocean coupler NOAA-GFDL/FMScoupler#145

Draft

hdrake mentioned this pull request Aug 19, 2024

Separating sea ice melt/formation from liquid precipitation field NOAA-GFDL/SIS2#214

Open

Revert changes to STALE_mct_cap and nuopc_cap

13b9960

As requested by @marshallward NOAA-GFDL#710 (comment)

hdrake marked this pull request as draft August 20, 2024 17:00

Merge branch 'MOM6-examples/dev/gfdl' into separate-seaice-melt

04cca2c

hdrake marked this pull request as ready for review December 11, 2024 22:47

Henri Drake added 2 commits December 11, 2024 14:48

Merge branch 'dev/gfdl' into separate-seaice-melt

ac855f9

Merge branch 'dev/gfdl' into separate-seaice-melt

791521c

Hallberg-NOAA reviewed Feb 1, 2025

View reviewed changes

config_src/drivers/FMS_cap/MOM_surface_forcing_gfdl.F90 Outdated Show resolved Hide resolved

Hallberg-NOAA reviewed Feb 1, 2025

View reviewed changes

src/core/MOM_forcing_type.F90 Outdated Show resolved Hide resolved

Hallberg-NOAA requested changes Feb 1, 2025

View reviewed changes

Henri Drake added 3 commits February 4, 2025 12:09

Fix indenting of fluxes@seaice_melt association check

5fd1689

Resolves @Hallberg-NOAA's comment (https://github.com/NOAA-GFDL/MOM6/pull/710/files#r1938257295)

Add parentheses to preserve old order of operations

f5443b3

Addresses @Hallberg-NOAA's comment: https://github.com/NOAA-GFDL/MOM6/pull/710/files#r1938256849

Merge branch 'dev/gfdl' into separate-seaice-melt

37d1385

breichl reviewed Mar 14, 2025

View reviewed changes

Hallberg-NOAA reviewed Apr 30, 2025

View reviewed changes

		if (associated(fluxes%heat_content_lprec)) then
		if (fluxes%lprec(i,j) > 0.0) then

Pass new seaice_melt field from SIS2 to MOM6 #710

Are you sure you want to change the base?

Pass new seaice_melt field from SIS2 to MOM6 #710

Uh oh!

Conversation

hdrake commented Aug 15, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

theresa-cordero commented Aug 19, 2024

Uh oh!

theresa-cordero commented Aug 19, 2024

Uh oh!

hdrake commented Aug 19, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

hdrake commented Aug 19, 2024

Uh oh!

theresa-cordero commented Aug 19, 2024

Uh oh!

hdrake commented Aug 19, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

hdrake commented Aug 20, 2024

Uh oh!

hdrake commented Aug 20, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

theresa-cordero commented Aug 20, 2024

Uh oh!

hdrake commented Aug 20, 2024

Uh oh!

marshallward commented Aug 20, 2024

Uh oh!

hdrake commented Aug 20, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov bot commented Aug 20, 2024

Codecov Report

Uh oh!

hdrake commented Dec 11, 2024

Uh oh!

hdrake commented Dec 11, 2024

Uh oh!

Uh oh!

Uh oh!

Hallberg-NOAA left a comment

Choose a reason for hiding this comment

Uh oh!

breichl Mar 14, 2025

Choose a reason for hiding this comment

Uh oh!

hdrake Apr 1, 2025

Choose a reason for hiding this comment

Uh oh!

Hallberg-NOAA Apr 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Hallberg-NOAA Apr 30, 2025

Choose a reason for hiding this comment

Uh oh!

Hallberg-NOAA Apr 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Hallberg-NOAA Apr 30, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Pass new `seaice_melt` field from SIS2 to MOM6 #710

Pass new `seaice_melt` field from SIS2 to MOM6 #710

hdrake commented Aug 15, 2024 •

edited

Loading

hdrake commented Aug 19, 2024 •

edited

Loading

hdrake commented Aug 19, 2024 •

edited

Loading

hdrake commented Aug 20, 2024 •

edited

Loading

hdrake commented Aug 20, 2024 •

edited

Loading

Hallberg-NOAA Apr 30, 2025 •

edited

Loading

Hallberg-NOAA Apr 30, 2025 •

edited

Loading