E3SM-Project / polaris

Testing and analysis for OMEGA, MPAS-Ocean, MALI and MPAS-Seaice
BSD 3-Clause "New" or "Revised" License
6 stars 13 forks source link

Add shared ocean framework for spherical convergence tests #119

Closed xylar closed 1 year ago

xylar commented 1 year ago

This merge adds some shared framework for spherical convergence tests with constant resolution meshes like cosine_bell and soon geostrophic and the various sphere_transport tests.

A new SphericalConvergenceForward parent class does most of the work involved in forward runs for convergence tests, including determining the approximate number of cells in the mesh (for constraining required resources), computing a time step based on the mesh resolution, and setting the run duration and output interval based on a config option.

A set of common (standardized) config options in the spherical_convergence section helps support this approach.

The cosine bell tests have been updated to use the shared framework, and the documentation as also been updated.

We no longer automatically add a task's config options to the config object -- this has to be done in configure(). Cosine bell was the only test so far that relied on this functionality and it did not work quite as expected. It is better to do it manually and control the order in which config options are added from different sources.

Checklist

xylar commented 1 year ago

Testing

All 4 cosine bell tests have been run successfully on Chrysalis with Intel and OpenMPI with this branch.

xylar commented 1 year ago

I'm keeping this in draft mode while I work on the geostrophic test because changes may be needed here to accommodate that test.

cbegeman commented 1 year ago

@xylar Since you already have a bit of clean-up in this PR, can you also change "visualizing" to "analyzing" in this line? https://github.com/E3SM-Project/polaris/blob/34fb2e76a7e5d58bc7dd17b758c8ea839f0c4418/polaris/ocean/tasks/cosine_bell/analysis.py#L13

xylar commented 1 year ago

@xylar Since you already have a bit of clean-up in this PR, can you also change "visualizing" to "analyzing" in this line?

https://github.com/E3SM-Project/polaris/blob/34fb2e76a7e5d58bc7dd17b758c8ea839f0c4418/polaris/ocean/tasks/cosine_bell/analysis.py#L13

Sure, I can take care of that.

xylar commented 1 year ago

@cbegeman, sorry of the delay. I was considering trying to fix config options for shared steps before this goes in, but that doesn't seem like a good idea at this point. The changes may be fairly large and this PR is needed for both geostrophic and sphere transport.

Please take a look and make sure everything has been updated as you had requested.

xylar commented 1 year ago

All 4 cosine bell test cases are still passing after these changes.

xylar commented 1 year ago

Hmm, some cosine bell changes snuck into #123 on a rebase. They were intended for this branch but will do no harm coming from that one instead, it's just a little confusing.

xylar commented 1 year ago

Thanks very much for the review and discussion, @cbegeman!