Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add vertical remap feature #4968

Merged
merged 22 commits into from
Jun 29, 2022

Conversation

cbegeman
Copy link
Contributor

@cbegeman cbegeman commented May 18, 2022

This PR adds the option for vertical Lagrangian remapping (VLR). The idea behind VLR is to allow the vertical interfaces between layers to evolve in a Lagrangian fashion, bypassing the Eulerian vertical advection scheme and removing the vertical CFL limit. The layer interfaces are then remapped to a target grid every interval of timesteps. Griffies et al. (2020) provides an overview of VLR.

This development includes the following features:

  • A new vertical remapping module mpas_ocn_vertical_remap.F
  • Adds PPR library as a submodule of the ocean component https://github.com/dengwirda/PPR, used to perform conservative remapping
  • Config option to turn remapping on/off: on = config_vert_advection_method = 'remap', off = config_vert_advection_method='flux-form' (default)
  • Config options for remapping: the order of remapping, the remapping interval, the remapping limiter
  • A diagnostic output file containing remapping-related variables
  • Computation of the Eulerian vertical velocity, which is nonzero at timesteps at which regridding and remapping occur
  • Vertical tracer advection budget terms must be computed after this Eulerian vertical velocity is computed. Thus, for the remapping case this computation is performed (as usual) in ocn_tracer_advection_mono_tend, where the vertical terms are zero, and repeated in diagnostics after remapping, where these terms may be nonzero.
  • Timer for remapping is named vertical remap. Roughly 20% of runtime is spent remapping when remapping occurs every timestep. However, in most realistic cases remapping at intervals of 3-5 timesteps are acceptable, reducing performance decreases to ~5%.
  • VLR is currently only supported for split-explicit time integrator
  • Regridding is hard-coded as z-star. Support for alternate grids is planned.

Further discussion and simulation results can be found at E3SM-Ocean-Discussion#6

[NML]
[BFB]

@cbegeman
Copy link
Contributor Author

cbegeman commented May 18, 2022

Running list of testing

Comparison with E3SM/master as of 175eec6d53 Merge remote-tracking branch 'OG/og/cleanup' (PR #4753)

  • SMS.T62_oQU120_ais20.MPAS_LISIO_TEST.anvil_gnu BFB

Comparison with E3SM/master as of b808d07 Merge branch 'beharrop/atm/fix-co2-diags-restarts' (PR #4941)

  • SMS.T62_oQU120_ais20.MPAS_LISIO_TEST.anvil_gnu (with openmpi)
  • SMS_D_Ld1.ne30pg2_r05_EC30to60E2r2.WCYCL1850.anvil_intel
  • PET_Ln3.T62_oEC60to30v3wLI.GMPAS-DIB-IAF-ISMF.anvil_intel
  • SMS_D.T62_oQU120_ais20.MPAS_LISIO_TEST.compy_pgi
  • PET_Ln3.T62_oEC60to30v3wLI.GMPAS-DIB-IAF-ISMF.compy_pgi
  • SMS.T62_oQU120_ais20.MPAS_LISIO_TEST.compy_intel

Compass

  • PR test suite BFB on anvil with intel/impi compilers

@xylar xylar added mpas-ocean BFB PR leaves answers BFB Stealth PR has feature which, if turned on, could change climate. fka FCC NML labels May 18, 2022
@cbegeman
Copy link
Contributor Author

@jonbob and @vanroekel Do either of you have recommendations for the 10-year B-case simulation recommended for stealth features under the new testing procedures? Thanks!

@vanroekel
Copy link
Contributor

@cbegeman my suggestion would be to use the water cycle v2 production configuration, i'd suggest the ne30pg2_EC30to60E2r2 is sufficient, but would definitely like @jonbob to chime in too.

@jonbob
Copy link
Contributor

jonbob commented May 19, 2022

I agree with @vanroekel that the low-res WC v2 configuration is correct for any longer comparison runs

@cbegeman
Copy link
Contributor Author

@jonbob My understanding from our slack conversation was that the 50-year G-case simulation I've already performed would be sufficient for this stealth feature. Let me know if your thoughts changed.

https://web.lcrc.anl.gov/public/e3sm/diagnostic_output/ac.cbegeman/scratch/anvil/mpas_analysis_output/20211020.GMPAS-JRA1p4.TL319_EC30to60E2r2.vlr.anvil.control/yrs46-50/

@jonbob
Copy link
Contributor

jonbob commented May 24, 2022

@cbegeman - your testing looks great. I'll do more testing before merging, but this is clearly ready and just has work its way through the backlog.

@cbegeman
Copy link
Contributor Author

FYI, I intend to add the commit from this branch ocn/add-vert-remap-design-doc, containing the design doc for VLR, after this PR is merged #4973:

@mark-petersen
Copy link
Contributor

@cbegeman thanks for your hard work on this. I am testing now. Do you have results from the idealized tests in Petersen et al. 2015? If not, I can run those and post results as part of my review. In particular, the overflow and internal gravity wave tests provide snapshots and quantitative measures for vertical advection.

@cbegeman
Copy link
Contributor Author

cbegeman commented May 27, 2022

@mark-petersen Thanks for testing! Yes, the rpe results from the baroclinic channel and internal wave test cases are:
image
and
image
Comparisons with different vlr options are located here under the Nov. 9, 2021 entry:
https://acme-climate.atlassian.net/wiki/spaces/PROBSLR/pages/130358226/Ocean+Model+Physics+Wetting-and-Drying

I don't have the overflow test case results. If you have that test case handy in legacy, feel free to give it a whirl.

Copy link
Contributor

@xylar xylar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved based on my careful review of E3SM-Ocean-Discussion#6, a quick look back through the code, and @cbegeman's testing.

@xylar
Copy link
Contributor

xylar commented May 31, 2022

@dengwirda, can you take a look at this as soon as possible?

@xylar xylar requested a review from philipwjones May 31, 2022 14:41
@xylar
Copy link
Contributor

xylar commented May 31, 2022

@philipwjones, could you make sure we're not missing OpenACC directives? If we are, suggestions would be welcome!

Copy link
Contributor

@dengwirda dengwirda left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Based on visual inspection + success with EC60to30 spin-up on cori.

@mark-petersen
Copy link
Contributor

Testing on badger. Gnu debug fails for RK4 and the threads test, when I add

/compass/ocean/tests/global_ocean/namelist.forward
@@ -1,4 +1,5 @@
+config_vert_advection_method = 'remap'

the results are:

module list
2) gcc/9.3.0   3) mvapich2/2.3   4) mkl/2020.0.4

00:02 PASS ocean_baroclinic_channel_10km_default
00:04 PASS ocean_baroclinic_channel_10km_threads_test
00:04 PASS ocean_baroclinic_channel_10km_decomp_test
00:05 PASS ocean_baroclinic_channel_10km_restart_test
00:38 PASS ocean_global_ocean_QU240_mesh
00:32 PASS ocean_global_ocean_QU240_PHC_init
00:33 PASS ocean_global_ocean_QU240_PHC_performance_test
01:07 PASS ocean_global_ocean_QU240_PHC_restart_test
01:04 PASS ocean_global_ocean_QU240_PHC_decomp_test
00:38 FAIL ocean_global_ocean_QU240_PHC_threads_test
00:46 PASS ocean_global_ocean_QU240_PHC_analysis_test
00:02 FAIL ocean_global_ocean_QU240_PHC_RK4_performance_test
00:02 FAIL ocean_global_ocean_QU240_PHC_RK4_restart_test
00:02 FAIL ocean_global_ocean_QU240_PHC_RK4_decomp_test
00:02 FAIL ocean_global_ocean_QU240_PHC_RK4_threads_test
00:00 PASS ocean_global_ocean_QUwISC240_mesh
00:00 PASS ocean_global_ocean_QUwISC240_PHC_init
00:35 PASS ocean_global_ocean_QUwISC240_PHC_performance_test
00:11 PASS ocean_ice_shelf_2d_5km_z-star_restart_test
00:18 PASS ocean_ice_shelf_2d_5km_z-level_restart_test
00:15 PASS ocean_ziso_20km_default
00:10 PASS ocean_ziso_20km_with_frazil

With the default config_vert_advection_method = 'flux-form' all tests pass

@cbegeman
Copy link
Contributor Author

cbegeman commented Jun 7, 2022

@mark-petersen Thanks for alerting me to this. The RK4 tests are expected to fail (we decided not to support RK4) but the threads test is a surprise to me. I'll look into it.

@cbegeman
Copy link
Contributor Author

cbegeman commented Jun 7, 2022

@mark-petersen Hmm, I'm having trouble replicating your error. I ran make gfortran DEBUG=True with the compass environment load_dev_compass_1.1.0-alpha.3_badger_gnu_mvapich.sh and the latest compass and that test passes

Test Runtimes:
00:51 PASS ocean_global_ocean_QU240_PHC_threads_test

Any idea why? Maybe @xylar has thoughts?

@xylar
Copy link
Contributor

xylar commented Jun 7, 2022

@cbegeman, I'm able to reproduce the error.

I'm seeing:

[THREAD 0001]CRITICAL ERROR: Trying to stop timer ppr when vertical remap is the most recently started timer.

@cbegeman, could it be that you're running with an older compass that doesn't see OPENMP=true by default? Maybe try updating to the latest compass master and see if you can reproduce the issue. Or explicitly build with OPENMP=true. Otherwise threading doesn't actually get tested.

@xylar
Copy link
Contributor

xylar commented Jun 7, 2022

My results are in:

/lustre/scratch4/turquoise/xylar/compass_1.1/test_20220607/ocean_nightly_vert_remap

in case they're helpful

@cbegeman
Copy link
Contributor Author

cbegeman commented Jun 7, 2022

@xylar Thanks! That's helpful. I got the test to pass by moving the vertical remap timer 67ca053 but for the threads test the vertical remap timer is not recorded in the log, only the ppr timer, which is within omp directives. Is that expected behavior?

@xylar
Copy link
Contributor

xylar commented Jun 8, 2022

@cbegeman, thanks for the fix in 67ca053. I must say I don't understand why that fixed the threading error but glad it did.

or the threads test the vertical remap timer is not recorded in the log, only the ppr timer, which is within omp directives. Is that expected behavior?

I don't know how timers relate to threading at all. @mark-petersen, do you have any idea why some timers show up and others don't with treading enabled

dengwirda and others added 21 commits June 27, 2022 14:39
- Add config option to turn remap on/off
- Add config option for advection, remap order
- Add config option for target grid
- Only call vert_transport_top if advection_method is flux-form
- Construct opts for remapping using config options
- vertAleTransportTop = 0 so no vertical velocity during
  momentum solve
- Add computation of vertVelocity from lagrangian
  layerThickness to diagnostics routines
- Add option for WENO slope limiter
- Fixup config name for vertical tracer adv order
- Add option for remapping every interval number of timesteps
- Update namelist entries
- Add checks on config options
@cbegeman cbegeman force-pushed the ocn/add-vertical-remap-feature branch from 6e34a3d to e30698f Compare June 27, 2022 20:39
@jonbob
Copy link
Contributor

jonbob commented Jun 27, 2022

test merge passes:

  • ERS.ne11_oQU240.WCYCL1850NS.chrysalis_intel
  • SMS_D_Ld3.T62_oQU120.CMPASO-IAF.chrysalis_intel

jonbob added a commit that referenced this pull request Jun 28, 2022
…4968)

Add vertical remap feature

This PR adds the option for vertical Lagrangian remapping (VLR). The
idea behind VLR is to allow the vertical interfaces between layers to
evolve in a Lagrangian fashion, bypassing the Eulerian vertical
advection scheme and removing the vertical CFL limit. The layer
interfaces are then remapped to a target grid every interval of
timesteps.

This development includes the following features:
* A new vertical remapping module mpas_ocn_vertical_remap.F
* Adds PPR library as a submodule of the ocean component used to perform
  conservative remapping
* Config option to turn remapping on/off:
    on  = config_vert_advection_method = 'remap'
    off = config_vert_advection_method='flux-form' (default)
* Config options for remapping: the order of remapping, the remapping
  interval and the remapping limiter
* A diagnostic output file containing remapping-related variables
* Computation of the Eulerian vertical velocity, which is nonzero at
  timesteps at which regridding and remapping occur
* Vertical tracer advection budget terms which must be computed after
  this Eulerian vertical velocity is computed. Thus, for the remapping
  case this computation is performed (as usual) in
  ocn_tracer_advection_mono_tend, where the vertical terms are zero, and
  repeated in diagnostics after remapping, where these terms may be
  nonzero.
* Timer for remapping named "vertical remapi". Roughly 20% of runtime is
  spent remapping when remapping occurs every timestep. However, in most
  realistic cases remapping at intervals of 3-5 timesteps are
  acceptable, reducing performance decreases to ~5%.
VLR is currently only supported for split-explicit time integrator.
Regridding is hard-coded as z-star. Support for alternate grids is
planned.

[NML]
[BFB]
@jonbob
Copy link
Contributor

jonbob commented Jun 28, 2022

also passes:

  • SMS.T62_oQU120_ais20.MPAS_LISIO_TEST.chrysalis_intel
  • SMS_D_Ld1.ne30pg2_EC30to60E2r2.WCYCL1850.chrysalis_intel.allactive-wcprod

merged to next

@jonbob jonbob merged commit 62b021e into E3SM-Project:master Jun 29, 2022
@jonbob
Copy link
Contributor

jonbob commented Jun 29, 2022

merged to master and expected NML DIFFs blessed

@xylar
Copy link
Contributor

xylar commented Jun 29, 2022

Yay! Congratulations, @cbegeman, this was a ton of work on your part.

@cbegeman
Copy link
Contributor Author

Thanks @xylar, @jonbob, @dengwirda, @mark-petersen and @philipwjones for working with me on this! I'm happy to see this in E3SM!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
BFB PR leaves answers BFB mpas-ocean NML Stealth PR has feature which, if turned on, could change climate. fka FCC
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants