Add explicit layerThicknessEdge variables

[cbegeman]

This PR replaces the existing layerThicknessEdge variable with two variables:

• layerThicknessEdgeMean refers to the mean of the neighboring cell-centered layerThickness
• layerThicknessEdgeFlux refers to the layerThicknessEdge used to flux momentum and tracers.

When config_thickness_flux_type = 'centered' layerThicknessEdgeFlux = layerThicknessEdgeMean, but when config_thickness_flux_type = 'upwind' layerThicknessEdgeFlux is upwinded.

Prior to this PR, when config_thickness_flux_type = 'upwind' layerThicknessEdge was upwinded everywhere. This PR uses layerThicknessEdgeFlux for horizontal advection terms and layerThicknessEdgeMean elsewhere, such as for mixing parameterizations. This change is in preparation for adding additional options for thickness fluxes.

This PR also removes layerThicknessEdge from global stats, since it did not offer much information not given by the global stats for layerThickness.

[BFB]

[xylar]

Additional discussion of this branch can be found at:
E3SM-Ocean-Discussion#12

[xylar]

@dengwirda, I'm not able to add you as an official reviewer, but please supply any comments.

[xylar]

@jonbob and @mark-petersen, do you have favorite bit-for-bit tests you'd recommend that we should run on this? I know I always ask this, but this PR seems particularly simple since it's largely just a variable name change. So a couple of tests to make sure nothing crazy has happened should be enough.

[jonbob]

@xylar - I like to run the developer tests on chrysalis that have active ocean. They're quick and cover a range of SMS and ERS:

• ERS.ne11_oQU240.WCYCL1850NS
• ERS_Ld5.T62_oQU120.CMPASO-NYF
• SMS_P12x2.ne4_oQU240.WCYCL1850NS.chrysalis_intel.allactive-mach_mods
• SMS.T62_oQU120_ais20.MPAS_LISIO_TEST
  This one is in the integration suite but adds a DEBUG test:
• SMS_D_Ld1.ne30pg2_r05_EC30to60E2r2.WCYCL1850

[xylar]

Perfect, thanks @jonbob! @cbegeman, I'll run these tomorrow unless you happen to get to it before I do.

[jonbob]

@xylar - I also run those for most PRs before merging, if you prefer to leave it until I test the merge

[xylar]

@cbegeman, I ran several tests on Anvil. The debug test (SMS_D_Ld1.ne30pg2_r05_EC30to60E2r2.WCYCL1850.anvil_intel) wasn't successful. That led me to look more carefully at globalStats, which is where the error was happening.

It turns out that this line:
https://github.com/cbegeman/E3SM/blob/034b20fad4014a0757b935b03b7ce4a036db88a8/components/mpas-ocean/src/analysis_members/mpas_ocn_global_stats.F#L1092

      volumeEdgeGlobal = sums(4)

is going to give us some real trouble. It's hard-coded that the 4th entry in the list of stats is layerThicknessEdge, so its sum is volumeEdgeGlobal. volumeEdgeGlobal is then used in several other stats we want to keep (those for normalVelocity, tangentialVelocity and normalizedAbsoluteVorticity).

So I think we're going to have to revert the changes to global stats (maybe by doing an interactive rebase and removing that commit).

Furthermore, it looks to me like we're going to need volumeEdgeFluxGlobal and volumeEdgeMeanGlobal because we compute the volume integral of normalVelocity and tangentialVelocity using layerThicknessEdgeFlux but normalizedAbsoluteVorticity using layerThicknessEdgeMean.

However, I wanted to check on that. It isn't clear to me that normalizedAbsoluteVorticity (and vorticity in general) should be getting computed with layerThicknessEdgeMean. Is that something we already discussed and I forgot?

[cbegeman]

@xylar Thanks for catching this. I propose that we add only the layerThicknessEdgeMean variable back into global_stats for those variables.

[xylar]

@cbegeman, I agree. I don't think there's any harm in volume-averaging normalVelocity and tangentialVelocity with layerThicknessEdgeMean. Please make sure layerThicknessEdgeFlux is not used anywhere in global stats. I believe it's currently used in 2 calculations that then get normalized later by volumeEdgeGlobal.

[cbegeman]

With the latest changes to mpas_ocn_global_stats.F, this branch passes SMS_D_Ld1.ne30pg2_r05_EC30to60E2r2.WCYCL1850.anvil_intel.

[xylar]

@cbegeman, I can rerun the rest of the tests I ran on Anvil tomorrow. But I think the fact that the debug test passes is a very encouraging sign!

[xylar]

I looked through the code in detail in E3SM-Ocean-Discussion#12.

I did a test merge of this PR with today's master. I then ran the following tests successfully on Anvil, using today's master as a baseline:

• compass ocean pr test suite
• ERS.ne11_oQU240.WCYCL1850NS.anvil_intel
• ERS_Ld5.T62_oQU120.CMPASO-NYF.anvil_intel
• SMS.T62_oQU120_ais20.MPAS_LISIO_TEST.anvil_intel
• SMS_P12x2.ne4_oQU240.WCYCL1850NS.anvil_intel.allactive-mach_mods
• SMS_D_Ld1.ne30pg2_r05_EC30to60E2r2.WCYCL1850.anvil_intel

Many of these are likely redundant with testing that @jonbob will do on Chrysalis but at least this covers another machine.

[xylar]

@mark-petersen, @sbrus89 and @dengwirda, if you could review this as soon as your able, that would be great!

[cbegeman]

Thanks for the prompt review and testing @xylar!

[mark-petersen]

Approving based on discussion in ocean-discussion and lengthy testing above. Thanks!

[sbrus89]

I did a test merge with today's master and ran the compass pr test suite on Anvil. All tests passed.

[jonbob]

test merge passes:

• ERS.ne11_oQU240.WCYCL1850NS.chrysalis_intel
• SMS_D_Ld1.ne30pg2_r05_EC30to60E2r2.WCYCL1850.chrysalis_intel

merged to next

[jonbob]

merged to master