Closed xylar closed 1 year ago
I will keep this in draft mode as I write the tutorial because more clean-up may be desirable and I don't want to have more PRs than necessary.
I ran 4 baroclinic channel test cases on Chrysalis and verified that they are BFB with a baseline from 2 days ago:
Test Runtimes:
00:07 PASS ocean/baroclinic_channel/10km/decomp_test
00:04 PASS ocean/baroclinic_channel/10km/restart_test
00:04 PASS ocean/baroclinic_channel/10km/threads_test
13:33 PASS ocean/baroclinic_channel/10km/rpe_test
Total runtime 13:49
@cbegeman, the tutorial is complete but relies on these changes. As they're pretty extensive, I'd appreciate you having a look. No need to test, as I've done that (but certainly no harm in more testing if you like).
This merge includes some clean-up to the baroclinic channel test group that I feel is important based on writing the tutorial.
First, this merge adds a framework-level function that computes
nx
andny
for a uniform, hexagonal planar mesh. The function takes into account the staggered nature of the hexagons in the y direction as corrected in #37.Then, in the baroclinic channel, instead of computing these properties in the test case and storing them in config options, they are computed using a framework-level function at setup in forward (to get the cell count) and then again at runtime in
initial_state
to make the mesh. Initial state storesnx
andny
as attributes in the initial state output file. The RPE analysis step then reads the attributes from the initial state file rather than config options.The reason for this change is that users should not modify these config options. But if a user does modify the domain size (
lx
andly
) either before setup or before running, this should result in changes innx
andny
. There is simply no good reason fornx
andny
to be config options other than the convenience of passing them between steps, which can be accomplished more cleanly with file attributes.The RPE analysis step previously used hard-coded values for the domain extent and tick marks. These have been updated to use
lx
andly
(though that is still not entirely accurate).This merge also changes the way that forward and analysis steps are added to the RPE test case. They need to be added both at init and setup because we want
polaris list --verbose
to show the default steps (require addition at init) and we want the steps to change at setup if a user supplies a different list of viscosities. Before this merge,polaris list --verbose
only showed theinitial_state
step in the RPE tests. Similar behavior in the compasscosine_bell
test case had proven confusing for developers so this same approach is used in both the compass and polaris versions of that test case.Checklist
api.md
) has any new or modified class, method and/or functions listedTesting
comment in the PR documents testing used to verify the changes