Enhance inactive top cells feature #840

cbegeman · 2021-04-04T19:10:50Z

Here, we extend the capability for Inactive top cells introduced by #825 to apply to additional model configurations besides those needed to run a basic ice shelf test case. More detail about the scope of this development is documented in #811.

The variable minLevelCell and related minLevelEdge* and minLevelVertex* variables are added by analogy with maxLevel* variables. By default, there are no inactive top cells and minLevel* = 1. The main code changes are to k-loop limits and designating surface quantities as those at minLevel*.

cbegeman · 2021-04-04T19:47:09Z

@vanroekel Do you know of a test case I can use to test the changes I made to ocn_tracer_short_wave_absorption_variable_tend with config_sw_absorption_type='ohlmann0'? It seems that this option requires forcing files. If not, do you know who would know about this?

cbegeman · 2021-04-04T21:46:18Z

src/core_ocean/shared/mpas_ocn_tracer_ecosys.F

@@ -1188,6 +1190,7 @@ subroutine ocn_tracer_ecosys_surface_flux_compute(activeTracers, ecosysTracers,
         BGC_forcing%SST(column) = activeTracers(indexTemperature,iLevelSurface,iCell)
         BGC_forcing%SSS(column) = activeTracers(indexSalinity,iLevelSurface,iCell)

+         ! the 1 indices may need to be changed to iLevelSurface


@maltrud Do you know whether the 1 index in ecosysTracers(ecosysIndices%*,1,iCell) should refer to the surface index (i.e., minLevelCell)?

The following six lines are a rearrangement of index order:
(k,iCell,iTracer) <--- (iTracer,k,iCell)
but only for a single column, and only for the top layer. I'm sure this is for the BGC interface API.

@cbegeman I agree that all 1 indices in the next six lines should be replaced with iLevelSurface. I think a shorter index name like kTop would be more readable. Perhaps kSurface.

@mark-petersen Thanks! These changes are in 9d189a2. With this commit, it passes the nightly test suite.

vanroekel · 2021-04-05T13:40:19Z

@cbegeman the shortwave absorption variable code is never tested (not since I. added it a number of years ago). I'd like to switch it on but it hasn't been high priority. For testing we would have to rerun the init phase of the COMPASS global ocean test case for each resolution and move them to the input data repo. Then we would have to do the testing by hand. I don't think that should be part of your responsibility in this PR. I'd propose a few possibilities

1. remove the minlevelcell changes from that file for now and revisit if we turn that functionality on later.
2. I could review the changes by visual inspection and approve that way.  I imagine that the changes in that module will be straight forward
3. We develop a test for this PR, I could help with this

I think my choice would be (2). What do you think?

cbegeman · 2021-04-05T14:37:31Z

TESTING

E3SM

Baseline: e3sm/master at 6db49a63ae

PASS comparison with baseline

SMS_D.T62_oQU120_ais20.MPAS_LISIO_TEST.cori-knl_intel
SMS_D.T62_oQU120_ais20.MPAS_LISIO_TEST.cori-knl_gnu
PEM_Ln9.ne30_oECv3.A_WCYCL1850S.cori-knl_intel
PET_Ln9.ne30_oECv3.A_WCYCL1850S.cori-knl_gnu
SMS_Ld1.ne30pg2_r05_EC30to60E2r2.A_WCYCL1850S_CMIP6.compy_intel.allactive-wcprod
SMS_PS.northamericax4v1pg2_WC14to60E2r3.A_WCYCL1850S_CMIP6.compy_intel.allactive-wcprodrrm
SMS_D.T62_oQU120_ais20.MPAS_LISIO_TEST.compy_pgi
SMS.T62_oQU120_ais20.MPAS_LISIO_TEST.compy_intel
PET_Ln9_P1024.ne30_oECv3.A_WCYCL1850S.compy_intel
SMS.T62_oQU120_ais20.MPAS_LISIO_TEST.anvil_gnu
SMS.T62_oQU120_ais20.MPAS_LISIO_TEST.anvil_intel
PEM_Ln9_P1024.ne30_oECv3.A_WCYCL1850S.anvil_intel
PET_Ln9_P1024.ne30_oECv3.A_WCYCL1850S.anvil_gnu
SMS_Ld1.ne30pg2_r05_EC30to60E2r2.A_WCYCL1850S_CMIP6.chrysalis_intel.allactive-wcprod @xylar
SMS.T62_oQU120_ais20.MPAS_LISIO_TEST.chrysalis_intel @xylar
PEM_Ln9_P1024.ne30_oECv3.A_WCYCL1850S.chrysalis_intel @xylar
PET_Ln9_P1024.ne30_oECv3.A_WCYCL1850S.chrysalis_intel @xylar
SMS_PS.northamericax4v1pg2_WC14to60E2r3.A_WCYCL1850S_CMIP6.chrysalis_intel.allactive-wcprodrrm @xylar

PASS (without comparison with baseline)

PET_Ln9.T62_oQU240.GMPAS-IAF.cori-knl_intel
PET_Ln3.T62_oEC60to30v3wLI.GMPAS-DIB-IAF-ISMF.cori-haswell_intel
PEM_Ln9.T62_oQU240.GMPAS-IAF.cori-haswell_gnu
PET_Ln3.T62_oEC60to30v3wLI.GMPAS-DIB-IAF-ISMF.compy_pgi
PET_Ln9.T62_oQU240.GMPAS-IAF.compy_pgi
PEM_Ln9.T62_oQU240.GMPAS-IAF.compy_pgi
PET_Ln9.T62_oQU240.GMPAS-IAF.anvil_gnu
PEM_Ln9.T62_oQU240.GMPAS-IAF.anvil_gnu
PET_Ln3.T62_oEC60to30v3wLI.GMPAS-DIB-IAF-ISMF.anvil_intel

Standalone

z_level_dev: encompasses the nightly regression suite and four additional ice-shelf test cases.
Baseline: ocean/develop and MPAS-Dev/compass/legacy + c1650df1 REMOVE LATER!!! Add z-leve dev. test suite

PASS

z_level_dev, Grizzly, intel
z_level_dev, Anvil, gnu @xylar
z_level_dev, Chrysalis, gnu @xylar
z_level_dev, Anvil, intel18 debug @xylar
z_level_dev, Chrysalis, intel debug @xylar

PASS except for ocean/sub_ice_shelf_2D/5km/iterative_init/ (non-bit-for-bit):

z_level_dev, Anvil, intel18 @xylar
z_level_dev, Chrysalis, intel @xylar

cbegeman · 2021-04-05T14:41:06Z

@vanroekel Thanks for this context. I think that you'll find option (2) to be reasonable, as the code changes are pretty straightforward and aren't extensive in these routines. The changes to shortwave_absorption_variable can be found in this commit: 9230b88. Let me know what you think.

vanroekel · 2021-04-05T14:53:09Z

The changes to shortwave_absorption_variable look good to me. I'm happy to approve those changes by visual inspection once the PR is ready to go.

Also, thanks for running the new Watercycle configuration on compy, I appreciate it!

xylar · 2021-04-05T15:23:59Z

@cbegeman, I know I still owe you some testing on Anvil and Chrysalis. I'll get to that as soon as I can, hopefully this evening.

src/core_ocean/shared/mpas_ocn_vmix.F

mattdturner

Approved via visual inspection and testing by @cbegeman and @xylar

cbegeman · 2021-04-06T21:19:53Z

@mattdturner Thanks for your review! I made that fix to mpas_ocn_vmix and pushed one more commit that resolves a merge conflict with the freshwater flux treatment in this commit 5aeccaa. I'll test that out shortly.

Add subpool calls Revert RK4 New=Cur subpool retrievals for minLevel* Use minLevelCell for velocityZonal,Meridional

cbegeman · 2021-04-08T22:04:35Z

@mark-petersen RK4 is now fixed. It passes nightly + z_level_dev standalone cases on Grizzly with intel17. I'll be retesting cvmix since we had some changes there.

cbegeman · 2021-04-09T16:11:36Z

cvmix tests single_column_model/sphere and single_column_model/planar are bit-for-bit with ocean/develop.

@xylar, it's ready for any testing you want to do. I'll start with Cori and Compy E3SM tests.

cbegeman · 2021-04-10T02:02:08Z

@xylar This branch still passes all compy and cori tests listed above as well as the nightly ocean test suite on Grizzly. I got a build error at [ 74%] Build target lnd on blues.

xylar

My testing all when great (see #840 (comment)). The one standalone test where there are diffs (ocean/sub_ice_shelf_2D/5km/iterative_init/) is clearly an optimization thing and it's not a configuration representative of anything we run production runs with so I'm fine with just "blessing" the changes.

cbegeman · 2021-04-14T01:52:39Z

@xylar Thank you for all your testing efforts!

mark-petersen

Reviewed visually and read past comments. Passes nightly regression suite on last commit
9d189a2 with intel 19 and gnu 5.3, both debug and optimized on grizzly, and bfb match with ocean/develop on both. Also, analytic comparison and convergence test for Redi terms are correct and match bfb with previous.

@cbegeman and @xylar thanks for your detailed work on this.

vanroekel

approving based on visual inspection and extensive testing from @xylar and @cbegeman

cbegeman · 2021-04-15T14:42:42Z

@vanroekel Thanks for all your input!

philipwjones

Previous comments addressed. Approve based on visual inspection, testing by others.

Enhance inactive top cells feature #840

Enhance inactive top cells feature MPAS-Dev#840

cbegeman commented Apr 4, 2021

View reviewed changes

xylar requested review from vanroekel, mattdturner, mark-petersen and xylar April 5, 2021 15:24

cbegeman added the Ocean label Apr 5, 2021

xylar requested a review from philipwjones April 5, 2021 15:24

mattdturner reviewed Apr 6, 2021

View reviewed changes

src/core_ocean/shared/mpas_ocn_vmix.F Outdated Show resolved Hide resolved

mattdturner approved these changes Apr 6, 2021

View reviewed changes

cbegeman force-pushed the ocean/enhance_minlevelcell_feature branch from a6dddf4 to 5ee6382 Compare April 6, 2021 20:58

cbegeman added 13 commits April 6, 2021 15:46

minLevel* changes to ocn_tracer_exponential_decay

942333c

minLevel* changes to ocn_tracer_DMS

531896a

minLevel* changes to ocn_tracer_ecosys

2e698a3

minLevel* changes to ocn_tracer_MacroMolecules

11c2121

minLevel* changes to ocn_tracer_ideal_age

5177995

minLevel* changes to ocn_tracer_interior_restoring

2280a98

minLevel* changes to ocn_filter_btr_mode_tend_vel

88ae8cc

minLevel* changes to ocn_tend_freq_filtered_thickness

2f0c2b1

minLevel* changes to ocn_tracer_short_wave_absorption_jerlov

4833ca5

minLevel* changes to ocn_tracer_short_wave_absorption_variable

97d835e

minLevel* changes to ocn_frazil_forcing

d0618e6

minLevel* changes to ocn_tracer_surface_flux_to_tend

57f09f9

minLevel* changes to ocn_tracer_nonlocalflux

82a35c4

cbegeman added 8 commits April 8, 2021 15:18

Fixup gm k-limit conditionals

4e2dbee

Fixup ocn_tidal_forcing

430ab93

Fixup ocn_tracer_DMS

ca5f52d

Fixup cvmix

318fb1c

minLevel* changes to RK4 timestepping

bcdcde3

Revert minLevel* changes to RK4: initialize *new = *cur

74d2acb

Revert minLevel* changes to RK4: diagnostic update

6da36d2

Fixup RK4:

f0c76b7

Add subpool calls Revert RK4 New=Cur subpool retrievals for minLevel* Use minLevelCell for velocityZonal,Meridional

cbegeman force-pushed the ocean/enhance_minlevelcell_feature branch from faf2897 to 9255673 Compare April 8, 2021 22:00

cbegeman and others added 2 commits April 12, 2021 15:40

Keep same vertical limits on kpp call

0816935

Fixup redi: change k to minLevelEdgeBot for flux terms

19dc7d9

cbegeman force-pushed the ocean/enhance_minlevelcell_feature branch from f9e02c3 to 19dc7d9 Compare April 12, 2021 21:24

xylar approved these changes Apr 13, 2021

View reviewed changes

Fixup ocn_tracer_ecosys

9d189a2

mark-petersen approved these changes Apr 14, 2021

View reviewed changes

mark-petersen mentioned this pull request Apr 14, 2021

Update mpas-source: Enhance inactive top cells feature E3SM-Project/E3SM#4219

Merged

vanroekel approved these changes Apr 15, 2021

View reviewed changes

philipwjones approved these changes Apr 15, 2021

View reviewed changes

mark-petersen added a commit that referenced this pull request Apr 20, 2021

Merge branch 'ocean/develop' into e3sm/develop

fddac40

Enhance inactive top cells feature #840

mark-petersen merged commit d2731fd into MPAS-Dev:ocean/develop Apr 20, 2021

cbegeman mentioned this pull request Apr 21, 2021

Enhance the option for inactive top cells cbegeman/MPAS-Model#3

Closed

mark-petersen added a commit to mark-petersen/MPAS-Model that referenced this pull request May 7, 2021

Merge branch 'ocean/develop' into e3sm/develop

d49cdf0

Enhance inactive top cells feature MPAS-Dev#840

cbegeman mentioned this pull request Jul 7, 2021

Fix bugs in inactive top cells feature E3SM-Ocean-Discussion/E3SM#5

Closed

cbegeman mentioned this pull request Oct 20, 2022

Bugfixes for ocean inactive top cells feature E3SM-Project/E3SM#5246

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Enhance inactive top cells feature #840

Enhance inactive top cells feature #840

cbegeman commented Apr 4, 2021

cbegeman commented Apr 4, 2021

cbegeman Apr 4, 2021

mark-petersen Apr 14, 2021

cbegeman Apr 14, 2021

vanroekel commented Apr 5, 2021

cbegeman commented Apr 5, 2021 •

edited by xylar

Loading

cbegeman commented Apr 5, 2021

vanroekel commented Apr 5, 2021

xylar commented Apr 5, 2021

mattdturner left a comment

cbegeman commented Apr 6, 2021

cbegeman commented Apr 8, 2021

cbegeman commented Apr 9, 2021

cbegeman commented Apr 10, 2021

xylar left a comment

cbegeman commented Apr 14, 2021

mark-petersen left a comment

vanroekel left a comment

cbegeman commented Apr 15, 2021

philipwjones left a comment

Enhance inactive top cells feature #840

Enhance inactive top cells feature #840

Conversation

cbegeman commented Apr 4, 2021

cbegeman commented Apr 4, 2021

cbegeman Apr 4, 2021

Choose a reason for hiding this comment

mark-petersen Apr 14, 2021

Choose a reason for hiding this comment

cbegeman Apr 14, 2021

Choose a reason for hiding this comment

vanroekel commented Apr 5, 2021

cbegeman commented Apr 5, 2021 • edited by xylar Loading

TESTING

cbegeman commented Apr 5, 2021

vanroekel commented Apr 5, 2021

xylar commented Apr 5, 2021

mattdturner left a comment

Choose a reason for hiding this comment

cbegeman commented Apr 6, 2021

cbegeman commented Apr 8, 2021

cbegeman commented Apr 9, 2021

cbegeman commented Apr 10, 2021

xylar left a comment

Choose a reason for hiding this comment

cbegeman commented Apr 14, 2021

mark-petersen left a comment

Choose a reason for hiding this comment

vanroekel left a comment

Choose a reason for hiding this comment

cbegeman commented Apr 15, 2021

philipwjones left a comment

Choose a reason for hiding this comment

cbegeman commented Apr 5, 2021 •

edited by xylar

Loading