Port merry-go-round test group from legacy

[cbegeman]

This PR ports the merry-go-round test group from legacy. The legacy test group was developed by @scalandr #108. The original test case is here named convergence_test and I added a default test that is just the lowest resolution forward run from the convergence_test.

The initial state was found to have only machine-precision level differences from the legacy version, and it also shows the same convergence behavior when compared with legacy using the same MPAS-Ocean code base.

The main differences between this implementation and that in legacy are

• using init_vertical_coord rather than hard-coding the vertical coordinate
• using matrix operations in initial_state to speed up computations
• minor tweaks to visualizations in viz and analysis steps

[cbegeman]

Note y-axis limits change, and scaling line is vertically shifted.

Old analysis plot:

New analysis plot:

[cbegeman]

Old viz plot:

New viz plot:

[cbegeman]

@scalandr The porting process went really smoothly!

Could you give initial_state a look? I removed your density initialization because it did not appear to be necessary.

Other than that, feel free to review or test with whatever rigor you prefer and have time for.

[cbegeman]

@mark-petersen Pinging you on this, since you reviewed the legacy version, if you want to take a look.

[scalandr]

Hi @cbegeman, I cloned the repo and tried to run the default test case but it tells me

  File "/gpfs/fs1/home/ac.scalandrini/repos/merryGoRound/compass/ocean/tests/merry_go_round/initial_state.py", line 138, in run
    temperature = temperature_background * xarray.ones_like(xCellDepth)
NameError: name 'temperature_background' is not defined
  test execution:      ERROR
  test runtime:        00:01
Test Runtimes:
00:01 FAIL ocean_merry_go_round_default
Total runtime 00:02
FAIL: 1 test failed, see above. 

[cbegeman]

@scalandr Thanks for catching that! I renamed the config options and missed that one. Now fixed by e005294

[mark-petersen]

Beautiful! Thank you @cbegeman. I tested successfully on badger:

report:

compass list |grep merry
 213: ocean/merry_go_round/default
 214: ocean/merry_go_round/convergence_test
(dev_compass_1.2.0-alpha.1) compass setup -p /usr/projects/climate/mpeterse/repos/E3SM/master/components/mpas-ocean -w $n/221115_merry -n 213 214
Setting up test cases:
  ocean/merry_go_round/default
  ocean/merry_go_round/convergence_test
target cores: 4
minimum cores: 4

salloc -N 1 -t 2:0:0 --qos=debug --reservation=debug --account=t22_ocean_time_step

cr
ocean/merry_go_round/default
compass calling: compass.ocean.tests.merry_go_round.default.Default.run()
  inherited from: compass.testcase.TestCase.run()
  in /turquoise/usr/projects/climate/mpeterse/repos/compass/master/compass/testcase.py

compass calling: compass.run.serial._run_test()
  in /turquoise/usr/projects/climate/mpeterse/repos/compass/master/compass/run/serial.py

Running steps: initial_state_5m, forward_5m, viz_5m
  * step: initial_state_5m
  * step: forward_5m
  * step: viz_5m

compass calling: compass.ocean.tests.merry_go_round.default.Default.validate()
  in /turquoise/usr/projects/climate/mpeterse/repos/compass/master/compass/ocean/tests/merry_go_round/default/__init__.py

  test execution:      SUCCESS
  test runtime:        00:04
Test Runtimes:
00:04 PASS ocean_merry_go_round_default
Total runtime 00:05
PASS: All passed successfully!
ba440:g:default$ cd ../
convergence_test/ default/
ba440:g:default$ cd ../convergence_test/
ba440:g:convergence_test$ cr
ocean/merry_go_round/convergence_test
compass calling: compass.ocean.tests.merry_go_round.convergence_test.ConvergenceTest.run()
  inherited from: compass.testcase.TestCase.run()
  in /turquoise/usr/projects/climate/mpeterse/repos/compass/master/compass/testcase.py

compass calling: compass.run.serial._run_test()
  in /turquoise/usr/projects/climate/mpeterse/repos/compass/master/compass/run/serial.py

Running steps: initial_state_1.25m, forward_1.25m, viz_1.25m, initial_state_2.5m, forward_2.5m, viz_2.5m, initial_state_5m, forward_5m, viz_5m, analysis
  * step: initial_state_1.25m
  * step: forward_1.25m
  * step: viz_1.25m
  * step: initial_state_2.5m
  * step: forward_2.5m
  * step: viz_2.5m
  * step: initial_state_5m
  * step: forward_5m
  * step: viz_5m
  * step: analysis

compass calling: compass.ocean.tests.merry_go_round.convergence_test.ConvergenceTest.validate()
  inherited from: compass.testcase.TestCase.validate()
  in /turquoise/usr/projects/climate/mpeterse/repos/compass/master/compass/testcase.py

  test execution:      SUCCESS
  test runtime:        01:07
Test Runtimes:
01:07 PASS ocean_merry_go_round_convergence_test
Total runtime 01:07
PASS: All passed successfully!

and the images were produced successfully. Thanks for your attention to detail on those images.

[scalandr]

Hi @cbegeman, I checked initial_state and everything looks fine to me. I tested successfully on Anvil.
As you saw, the default advection types for this test case are

config_vert_tracer_adv_order=2
config_horiz_tracer_adv_order=2
config_monotonic=.false.

I was wondering if we have to test what happens with a third-order advection scheme or with monotonic on. This goes beyond the test case itself, but I wanted to ask for your opinion.

[cbegeman]

@scalandr I'm glad you asked that question. It led me to realize that the namelist files do not have the updated namelist options so I fixed that here: 72ac942. The results I showed earlier are with the monotonic scheme on.

Interestingly, when the monotonic scheme is on, the results are the same for 2nd and 3rd order. When the monotonic scheme is off, we get lower error overall. Changing the order of accuracy doesn't change the slope but does reduce the error:

2nd order, monotonic

2nd order, non-monotonic

3rd order, non-monotonic

[scalandr]

Hi @cbegeman, from your plots is clear that monotonic has, as expected, an impact on the convergence rate. I re-did your plots and added an order 1 line, so that is even more clear that the global order of the model goes down to 1 if we move from a standard finite volume advection discretization to a monotonic one. This result is expected, but is nice to see it confirmed in this test.

monotonic advection:

non-monotonic advection:

[scalandr]

Hi @cbegeman, I think it all looks great.
I believe we should keep config_flux_limiter = 'none' as default exactly as it is right now. I am happy that this test case will be in the official compass list!

[cbegeman]

@scalandr Thank you so much for your thorough review! In the spirit of code coverage, we might consider adding a version that is monotonic in addition to the non-monotonic test later on. But I'm happy to merge this PR with only the non-monotonic tests.

[xylar]

@cbegeman, wonderful work as always! @scalandr, again I appreciate your work on the original test case!

I'm testing this now but based on looking through the code, there are a few changes it would be nice to have.

The only one I feel strongly about is the mask, which I don't think is very easy to follow with the current logic based on cell index.

[xylar]

My actual runs of the code on Chrysalis with Intel and Intel-MPI went very smoothly. The viz and analysis plots look great!

[cbegeman]

@xylar Thanks for the suggestions! Ready for your re-review when you have a chance.

[xylar]

The updated code looks perfect to me!