Clarify DSS weights and fix DSS buffer bug

[dennisYatunin]

This PR extends the DSS machinery to handle 3D weights alongside 2D weights, and it fixes the weights for deep-atmosphere grids. The weights are now always consistent with the metric determinant $J$, ensuring that DSS does not violate conservation of scalars or vorticity in ClimaAtmos. Update: This turned out not to be necessary (see comment below).

In addition, this PR adds a unit test for conservation and enforces consistency of function/variable names, ensuring that the weights are always called dss_weights and updating docstrings where needed. It also fixes a preexisting bug in DSS buffer allocation for, e.g., Covariant12Vectors, which are transformed into UVWVectors when DSSed in a 3D space, and so have Nf = 2 for the input data and Nf = 3 for the buffer. There was previously a line in the Topologies module that assumed these values of Nf are equal.

• Code follows the style guidelines OR N/A.
• Unit tests are included OR N/A.
• Code is exercised in an integration test OR N/A.
• Documentation has been added/updated OR N/A.

[dennisYatunin]

@akshaysridhar, could you also try out the conservation check from CliMA/ClimaAtmos.jl#3422 with this PR? I think that's currently the only test where we would expect to see a noticeable difference.

[charleskawczynski]

Overall I think this is a good improvement, but I have a few comments that I think need addressed:

• Name bikeshedding: dss_weights vs local_dss_weights? I don't have strong opinions, but I'd like to push towards consistent names if possible, and we do already have local_geometry
• Restarts: we should add center_dss_weights and face_dss_weights to InputOutput readers / writers, so that simulations can properly be restarted.
• Tests: is there some unit test we can add / update to show the impact of these changes? (e.g., a convergence test like it the other deep atmosphere fix?)

[dennisYatunin]

Thanks @charleskawczynski! I will try to set up a deep-atmosphere conservation unit test like I had for the column integrals. As for the other two points:

• The local_ in local_geometry is to distinguish it from global_geometry. There is no such analogue for DSS weights, of which we only have one instance. DSS is also often referred to as "global assembly", so it might be a bit confusing to call the DSS weights "local".
• The InputOutput reader already saves all the information it needs for ExtrudedFiniteDifferenceGrids, including the deep keyword argument, and the corresponding writer just calls the constructor. There is no need to add additional information for restarts.

[charleskawczynski]

Thanks @charleskawczynski! I will try to set up a deep-atmosphere conservation unit test like I had for the column integrals. As for the other two points:

Great, thanks!

• The local_ in local_geometry is to distinguish it from global_geometry. There is no such analogue for DSS weights, of which we only have one instance. DSS is also often referred to as "global assembly", so it might be a bit confusing to call the DSS weights "local".

Sounds good to me.

• The InputOutput reader already saves all the information it needs for ExtrudedFiniteDifferenceGrids, including the deep keyword argument, and the corresponding writer just calls the constructor. There is no need to add additional information for restarts.

Ah, right, that outer constructor will just create these dss weights. Sounds good.

Overall looking very good!

[dennisYatunin]

After adding the conservation test, I have found that changing to 3D weights does nothing to improve conservation in deep atmospheres (the maximum error was 13 times machine epsilon with both 2D and 3D weights). This is because the radial distance r and the vertical metric derivative ∂z/∂ξ₃, both of which factor into the DSS weights, are continuous across element boundaries, so their contributions to the weights on either side of each boundary cancel out.

Based on this result, I've removed the part of this PR that modifies the weights to 3D, retaining the old behavior of using the horizontal weights for extruded fields, and I've added a comment to explain why we are allowed to do this. I'll keep the remaining parts of this PR, like the buffer bugfix, conservation test, and improvements in naming consistency.

[charleskawczynski]

It also fixes a preexisting bug in DSS buffer allocation for, e.g., Covariant12Vectors, which are transformed into UVWVectors when DSSed in a 3D space, and so have Nf = 2 for the input data and Nf = 3 for the buffer. There was previously a line in the Topologies module that assumed these values of Nf are equal.

Can you please point to where this fix is? I don't see it.

Also, is this news-worthy? And is it at all behavior changing (thinking about reproducibility tests)?

[dennisYatunin]

@charleskawczynski, here is the relevant line on main:

ClimaCore.jl/src/Topologies/dss.jl

Line 104 in 4f97a51

Nf = DataLayouts.ncomponents(data)

And here is the new version:

ClimaCore.jl/src/Topologies/dss.jl

Line 115 in c55317c

Nf = DataLayouts.typesize(T, TS)

I think the value of Nf used here needs to match the DSS buffer, rather than the incoming data.

Now that I've removed the use of 3D weights for deep atmospheres, nothing in this PR should be behavior changing. I'm not sure if any of the remaining housekeeping changes, like the internal variable renaming and the new unit test, are newsworthy.