E3SM-Project / E3SM

Energy Exascale Earth System Model source code. NOTE: use "maint" branches for your work. Head of master is not validated.
https://docs.e3sm.org/E3SM
Other
336 stars 338 forks source link

Add parametrized tidal mixing effects to ice shelf basal melt #6306

Closed irenavankova closed 2 weeks ago

irenavankova commented 2 months ago

An implementation of prescribed tidal effect on ice shelf basal melting following (Jourdain et al., 2019):

https://doi.org/10.1016/j.ocemod.2018.11.001

The tidal flow speeds used to produce a forcing data file are calculated from the CATS 2008 (an update of Padman et al., 2002). When the file is available and compset TMIX on, the Jourdain formula is used. The main change is that what used to be a constant config_land_ice_flux_rms_tidal_velocity is now a spatially variable (horizontal) field.

Related Ocean Discussion is here:

https://github.com/E3SM-Ocean-Discussion/E3SM/pull/76

[NML] [BFB]

xylar commented 2 months ago

@jonbob, I've marked this as "Stealth" but I'm not sure if that is correct. Let me know what you think. I'm going to run an Icos G-case with this configuration for 10 years and see what we get. I'll compare with the same configuration without tidal mixing. Hopefully, that's sufficient as a test of this capability as a stealth feature. I'm also going to figure out where to add an appropriate test.

xylar commented 2 months ago

@jonbob, what do you think of adding:

            "SMS_P480_Ld5.TL319_IcoswISC30E3r5.GMPAS-JRA1p5-DIB-PISMF-TMIX.mpaso-jra_1958",

to either e3sm_ocnice_stealth_features or e3sm_ocnice_extra_coverage?

jonbob commented 2 months ago

@xylar -- I don't think it's exactly a stealth feature since it's more like a new separate configuration, unless the goal is to make this default behavior

xylar commented 2 months ago

@jonbob, it's certainly conceivable that we could make this the default behavior for Polar runs. That's the sense in which I was thinking of it as a stealth feature.

jonbob commented 2 months ago

I think in our current definition, this is a separate configuration and we can remove the stealth label and add a new test to extra coverage. But it's still important to document what impact it has as part of the PR

xylar commented 2 months ago

@irenavankova, I hope you don't mind that I pushed a test.

jonbob commented 4 weeks ago

@irenavankova -- I ran the scripts that make the bld files consistent with Registry and they came up with some differences in namelist_definition_mpaso.xml:

--- a/components/mpas-ocean/bld/namelist_files/namelist_definition_mpaso.xml
+++ b/components/mpas-ocean/bld/namelist_files/namelist_definition_mpaso.xml
@@ -1608,25 +1608,25 @@ Default: Defined in namelist_defaults.xml

 <entry id="config_land_ice_flux_tidal_Jourdain_alpha" type="real"
        category="land_ice_fluxes" group="land_ice_fluxes">
-Constant in parameterization of tidal velocity used in computing the sub-ice-shelf friction velocity
+alpha from parameterization of tidal velocity by Jourdain et al., 2019 - Equation 9, alpha = 0.777 when TMIX compset on

-Valid values: Any positive real number
+Valid values: Any non-negative real number
 Default: Defined in namelist_defaults.xml
 </entry>

 <entry id="config_land_ice_flux_tidal_Jourdain_A0" type="real"
        category="land_ice_fluxes" group="land_ice_fluxes">
-Constant in parameterization of tidal velocity used in computing the sub-ice-shelf friction velocity
+A0 from parameterization of tidal velocity by Jourdain et al., 2019 - Equation 9, A0 = 0.656 when TMIX compset on

-Valid values: Any positive real number
+Valid values: Any non-negative real number
 Default: Defined in namelist_defaults.xml
 </entry>

 <entry id="config_land_ice_flux_tidal_Jourdain_U0" type="real"
        category="land_ice_fluxes" group="land_ice_fluxes">
-Constant in parameterization of tidal velocity used in computing the sub-ice-shelf friction velocity
+U0 from parameterization of tidal velocity by Jourdain et al., 2019 - Equation 9, U0 = 0.003 m/s when TMIX compset on

-Valid values: Any positive real number
+Valid values: Any non-negative real number
 Default: Defined in namelist_defaults.xml
 </entry>

Can you please help me resolve the differences?

jonbob commented 2 weeks ago

passes:

with expected NML DIFFs. Also passes:

which is the new test introduced by this PR

merged to next

jonbob commented 2 weeks ago

verified that all new files are available on the inputdata repo and have correct permissions

jonbob commented 2 weeks ago

@irenavankova and @xylar -- it looks like this PR was non-BFB on pm-cpu using the gnu compiler. From looking at the changes, it can only be order-of-operations level though of course with the model any little change becomes larger. How do you want to handle this? It is only the one machine and one compiler. I tested it on chrysalis with gnu and did not see any DIFFs and overnight testing with intel passed

xylar commented 2 weeks ago

Oh, that's odd. I guess since you merged this, you think it's okay. I guess we'll need to update baselines for pm-cpu? I think that's likely okay since it was BFB on other machines.

xylar commented 2 weeks ago

Thanks for the help with this, @jonbob and thanks for working so hard on it, @irenavankova!

jonbob commented 2 weeks ago

I did go ahead and merge it because it passed on our production platforms and compilers -- it seems like a one-off. I can just bless those DIFFs, or if you want we can rework the code section that actually does the work and try to get it BFB on pm-cpu_gnu? I'm guessing it would be possible using the new configs that were added? Or decrease the level of optimization for the affected file?

xylar commented 2 weeks ago

I think we should just bless the difference. The only simulations where this is going to have any impact at all (apart from ensemble variability) are those with prognostic ice-shelf melting and we don't have any production simulations going like that right now.

jonbob commented 2 weeks ago

OK, I blessed the DIFFs for the PISMF tests on pm-cpu. There is another new DIFF that doesn't make sense to me:

ERS_Ld5.TL319_oQU240wLI_ais20.MPAS_LISIO_JRA1p5.pm-cpu_gnu.mpaso-ocn_glcshelf
xylar commented 2 weeks ago

That's also PISMF, but through the coupler so it makes sense to me. The friction velocity, modified by this PR, will be used there as well.

jonbob commented 2 weeks ago

Thanks @xylar -- I'll bless that test as well