Have `XiosRead_test` read output of `XiosWrite_test` #999

joewallwork · 2025-11-24T16:02:45Z

Have `XiosRead_test` read output of `XiosWrite_test`

Fixes #990
~~Merges into #996~~

Task List

Defined the tests that specify a complete and functioning change (It may help to create a design specification & test specification)
Implemented the source code change that satisfies the tests
Documented the feature by providing worked example
Updated the README or other documentation
Completed the pre-Request checklist below

Test Description

From the issue:

Currently, the XIOS write test checks that a file with the expected name is created, but doesn't check its contents.

To facilitate this change, XiosWrite_test was set to be a dependency of XiosRead_test in core/test/CMakeLists.txt. As a result, if any subset of the test suite is run that includes both of them is run then the write test will come first.

Change Description

The main code change required to support the revised testing was to set the values for the x- and y- dimensions of each domain. These aren't actually used in the model so I just set them to coincide with the local index values. I also needed to correct the names for the axis dimensions. With these changes, the XiosWrite_test produces output that's accepted by the XiosRead_test.

As part of the PR, I dropped the split frequency functionality from the test with the intention of re-enabling it in a follow-up PR that addresses #898.

Documentation Impact

None

Pre-Request Checklist

The requirements of this pull request are fully captured in an issue or design specification and are linked and summarised in the description of this PR
No new warnings are generated
Methods and Tests are commented such that they can be understood without having to obtain additional context
This PR/Issue is labelled as a bug/feature/enhancement/breaking change
This change conforms to the conventions described in the README

joewallwork · 2025-11-25T10:08:39Z

It turns out that the reason the tests fail is that XIOS appends the names of dimensions associated with domains with the domain name, e.g., x becomes x_HDomain and yvertex becomes yvertex_VertexDomain. The same behaviour doesn't hold for axes. I've contacted the developers to ask if this could be changed, perhaps with a switch. Otherwise, we'd have to either (a) replace all uses of domains with axes, or (b) rename the dimensions as something like x_h, y_h, x_vertex, y_vertex (and x_cg and y_cg would already be okay), which would be inconsistent with the naming scheme up until now.

… splitting for now

joewallwork · 2025-12-01T12:09:14Z

[Rebased on top of issue850_xios-restructure.]

timspainNERSC

I think the canonical and alternate names in ModelArrayDetails.cpp should be swapped. Everything else looks ready to be merged.

timspainNERSC · 2025-12-01T13:15:28Z

core/src/discontinuousgalerkin/ModelArrayDetails.cpp

+    { ModelArray::Dimension::X, { "xdim", "x_dim", 0, 0, 0 } },
+    { ModelArray::Dimension::Y, { "ydim", "y_dim", 0, 0, 0 } },
+    { ModelArray::Dimension::XVERTEX, { "xvertex", "x_vertex", 1, 1, 0 } }, // defined as x + 1
+    { ModelArray::Dimension::YVERTEX, { "yvertex", "y_vertex", 1, 1, 0 } }, // defined as y + 1


I'd suggest using x_dim as the primary name, as that is the name that will be used in files written using the legacy code path. It would be good if the XIOS code path could read these files by default.

Okay, I've addressed this in fa86c47. I also changed old_names to be alt_names and updated accordingly in 183435f.

joewallwork · 2025-12-02T14:51:25Z

Okay after quite a bit of debugging I figured out the errors in the integration tests.

…n-read-take3

timspainNERSC

I'm going to approve this, but everywhere you have changed x& y to xdim & ydim should really be changed to x_dim & y_dim, since these are the primary names for these dimensions. That they were not changed is my oversight when I originally changed the names, so thanks for fixing that :).

timspainNERSC · 2025-12-04T09:50:28Z

core/src/include/gridNames.hpp

+static const std::string xName = "xdim";
+static const std::string yName = "ydim";


I think these should be x_dim and y_dim, the primary names. I must not have changed this when making the original move from x,y to xdim, ydim.

Done in 1a41099.

timspainNERSC · 2025-12-04T09:51:44Z

core/test/ParaGridIO_input_test.py

+    xDim = ncFile.createDimension("xdim", nx)
+    yDim = ncFile.createDimension("ydim", ny)


Again, these should be the primary names and should have been changed when I moved from x, y to xdim, ydim.

Done in 1a41099.

timspainNERSC · 2025-12-04T09:52:56Z

physics/test/era5_test_data.py