Add convenience constructors for grids

[charleskawczynski]

Supersedes #1567.

I've made all arguments optional, so that users can call, for example, ExtrudedCubedSphereGrid() for an extruded cubed sphere grid.

[charleskawczynski]

I don't like that ColumnGrid returns a FiniteDifferenceGrid, maybe we should bike-shed that name a bit more.

[Sbozzolo]

There are things that look odd to me. For example, I am looking at the sphere, the vertical resolution is hidden, while much less useful arguments like boundary_names are immediately customizable.

Maybe we should tackle this by collecting requirements and coming up with a proposal before implementing it, while also asking for opinions by users/scientists too.

In this proposal, we should indentify our goals with the convenience constructors and understand what is important to be immadiately expose and settable and what is not. ClimaLand already implements something similar (Domains), can we come up with something that Land could use instead of reimplmenting it?

The broader problem I would like to solve is that the abstractions provided by ClimaCore for computational grids are too low level for pretty much all practical applications. Maybe we don't really need convenience constructors, but another layer of abstraction that connects more with real applications (while leaving the lower level abstraction still accessible). So, science applications like Atmos and Land would use this level of abstraction instead of dealing with the more primitives objects.

Here was my attempt to identify what matters to scientists in ClimaAtmos:

CliMA/ClimaAtmos.jl#2147

For spheres, all atmos uses is this set of parameters and options:

            nh_poly
            h_elem
            z_elem
            radius
            z_max
            z_stretch
            dz_bottom
            dz_top
            float_type
            enable_bubble
            topography

As we can see, there is no real reference to meshes, domains, topology, quadrature, and so on, which we could use as a guide to target our implementation.

(I realize it is not easy to strike the correct balance and design the best solution)

[charleskawczynski]

There are things that look odd to me. For example, I am looking at the sphere, the vertical resolution is hidden while much less useful arguments like boundary_names are immediately customizable.

How is vertical resolution hidden? z_elem::Integer = 10 is a kwarg, just like boundary_names.

Maybe we should tackle this by collecting requirements and coming up with a proposal before implementing it, while also asking for opinions by users/scientists too.

In this proposal, we should indentify our goals with the convenience constructors and understand what is important to be immadiately expose and settable and what is not. ClimaLand already implements something similar (Domains), can we come up with something that Land could use instead of reimplmenting it?

I don't think that this is something that needs requirements, these are convenience constructors, not new data structures. I don't see what https://github.com/CliMA/ClimaLand.jl/blob/main/src/shared_utilities/Domains.jl is solving. I'd probably suggest that those data structures be removed (by effectively inlining all of that code). Doing so would probably greatly simplify the land code, its ClimaCore interface, and potentially performance (since there are a lot of abstract container types). This is the same exercise I performed in TurbulenceConvection (TC), and the only reason that I didn't remove TC's Grid abstraction was because Nv was not in the type space, and TC's grid basically helped prevent loads of type instabilities by storing Nv in the type space. Now that `` is closed, TC's grid abstraction is no longer needed (and should probably be removed).

The broader problem I would like to solve is that the abstractions provided by ClimaCore for computational grids are too low level for pretty much all practical applications. Maybe we don't really need convenience constructors, but another layer of abstraction that connects more with real applications (while leaving the lower level abstraction still accessible). So, science applications like Atmos and Land would use this level of abstraction instead of dealing with the more primitives objects. ....

I don't think that defining higher level abstractions than what ClimCore currently provides is well defined at the layer of ClimaCore.

This looks like a really separate matter, and I'd rather keep the scope of this from creeping. So, I think the first question is, do we want to support convenience constructors for grids or not?

We can discuss CliMA/ClimaAtmos.jl#2147 separately, but I think that is a separate discussion.

[Sbozzolo]

There are things that look odd to me. For example, I am looking at the sphere, the vertical resolution is hidden while much less useful arguments like boundary_names are immediately customizable.

How is vertical resolution hidden? z_elem::Integer = 10 is a kwarg, just like boundary_names.

Ah, yes, you are right. I was thinking about resolution in terms of dz, so this didn't register to me.

Maybe we should tackle this by collecting requirements and coming up with a proposal before implementing it, while also asking for opinions by users/scientists too.

In this proposal, we should indentify our goals with the convenience constructors and understand what is important to be immadiately expose and settable and what is not. ClimaLand already implements something similar (Domains), can we come up with something that Land could use instead of reimplmenting it?

I don't think that this is something that needs requirements, these are convenience constructors, not new data structures. I don't see what https://github.com/CliMA/ClimaLand.jl/blob/main/src/shared_utilities/Domains.jl is solving. I'd probably suggest that those data structures be removed (by effectively inlining all of that code). Doing so would probably greatly simplify the land code, its ClimaCore interface, and potentially performance (since there are a lot of abstract container types). This is the same exercise I performed in TurbulenceConvection (TC), and the only reason that I didn't remove TC's Grid abstraction was because Nv was not in the type space, and TC's grid basically helped prevent loads of type instabilities by storing Nv in the type space. Now that `` is closed, TC's grid abstraction is no longer needed (and should probably be removed).

This is the problem. What ClimaCore provides is impractical to use, so every application (from test suites to other packages) had to define additional abstractions/functions to work with ClimaCore.

The broader problem I would like to solve is that the abstractions provided by ClimaCore for computational grids are too low level for pretty much all practical applications. Maybe we don't really need convenience constructors, but another layer of abstraction that connects more with real applications (while leaving the lower level abstraction still accessible). So, science applications like Atmos and Land would use this level of abstraction instead of dealing with the more primitives objects. ....

I don't think that defining higher level abstractions than what ClimCore currently provides is well defined at the layer of ClimaCore.

This looks like a really separate matter, and I'd rather keep the scope of this from creeping. So, I think the first question is, do we want to support convenience constructors for grids or not?

I think the problem we are trying to solve with this PR is simplifying creating computational domains.

I think that this is well within in the scope of ClimaCore and I think this was one of Simon's goals with introducing Grids (besides the technical point of making Spaces mutable).

Several numerical libraries have an abstraction for the computational grid. E.g.,

• trixi
• oceananingans
• FEniCS

And it could be that the solution is simply defining convenience constructors.

[charleskawczynski]

Ah, yes, you are right. I was thinking about resolution in terms of dz, so this didn't register to me.

👍🏻

This is the problem. What ClimaCore provides is impractical to use, so every application (from test suites to other packages) had to define additional abstractions/functions to work with ClimaCore.

I'm not sure I agree-- our spaces are the highest level abstraction, and I think they're practical. The ClimaCore test suite does define some helpers, but that's mostly for setting up examples and reusing code between tests. I think Atmos uses the ClimaCore data structures just fine. Regardless, let's keep the introduction of new data structures/abstractions to a separate task.

The broader problem I would like to solve is that the abstractions provided by ClimaCore for computational grids are too low level for pretty much all practical applications. Maybe we don't really need convenience constructors, but another layer of abstraction that connects more with real applications (while leaving the lower level abstraction still accessible). So, science applications like Atmos and Land would use this level of abstraction instead of dealing with the more primitives objects. ....

There's a few things to unpack here. First, low level code does have advantages (e.g., less overhead, and simpler/more direct behavior). Also, it can be pretty painful to backpedal from (i.e., remove support for) higher level abstractions to make breaking changes after users have adopted them.

User-friendliness is, IMHO, a bit subjective. Regarding computational grids are too low level for pretty much all practical applications, if grids/spaces are too low level, what information is missing from these data structures? If it's just the matter of the boilerplate needed to make spaces in the first place, then the convenience constructors should help. But, if more data structures are needed, I'd first like to know what information is missing that users want, before defining new data structures that reorganize already existing information in spaces.

I think the problem we are trying to solve with this PR is simplifying creating computational domains.

If that's the case, then this PR should help.

I think that this is well within in the scope of ClimaCore and I think this was one of Simon's goals with introducing Grids (besides the technical point of making Spaces mutable).

This is Simon's PR that added grids: #1487, it was implemented to fix #1467 and #1120.

Several numerical libraries have an abstraction for the computational grid. E.g.,

• trixi
• oceananingans
• FEniCS

And it could be that the solution is simply defining convenience constructors.

Yes, and we've had similar abstractions for a while-- our mesh is similar to the FEniCS and trixi links and our mesh/space is similar to the oceananigans one (as that grid also includes topology).

This goes back to my earlier question: what information is missing? All the links above are from examples, which tend to use convenience constructors for pedagogical/demonstrative reasons. Oceananigans still have relatively complex looking grids that are probably not called from examples (https://github.com/CliMA/Oceananigans.jl/blob/fbf05fb72624c4436c854705dc59f80a9f62a4d2/src/Grids/latitude_longitude_grid.jl#L3-L70 😵‍💫)

Reading through CliMA/ClimaAtmos.jl#2147 (comment), I think that one thing that we could do is add more high-level getter-helpers that grab things from internals-- and I think that this is a perfectly fine/safe thing to do. For example, we could define z_max(space::AbstractSpace) = space.vertical_topology.mesh.domain.coord_max.z.

[charleskawczynski]

@Sbozzolo, I think that this may be ready for another round of review

[Sbozzolo]

Thank you. I think this looks good and is very useful.

The next step here would be to add documentation and examples so that is clear how to use these new functions. Given that these are user-facing, I think we should target a level of documentation and explanations useful to someone who is not very familiar with ClimaCore (e.g., what does h_elem mean?).

[charleskawczynski]

I don't really like the name Nq, I think I prefer N_poly or N_quad_points. Thoughts?

[Sbozzolo]

I don't really like the name Nq, I think I prefer N_poly or N_quad_points. Thoughts?

I agree

[charleskawczynski]

I see the atmos docs say that Nq is the number of quadrature points, I'll use n_quad_points.

[Sbozzolo]

These constructors look good to me and I am looking forward seeing them used.

I think it's very important that we add examples and start converting code to using these constructors as much as possible so that people learn that this is how they can build computational grids.

So, this PR should also:

• Add some examples on how to use the constructors.
• Add a NEWS entry that also quickly highlight how to use this (vs the old way)

I think that these functions are very useful and also very user-facing, so I would like to see the docstrings expanded to explain the various terms in a way that is accessible to someone who is not familiar to ClimaCore.

[charleskawczynski]

I think it's very important that we add examples and start converting code to using these constructors as much as possible so that people learn that this is how they can build computational grids.

Why do we want to spread their use? One concern I have with that is that all of these constructors are coupled to all of the components they build on-top of. So, changes to any layer beneath it requires changes for these constructors. This is not dissimilar to dependencies, lighter dependencies tend to be easier to maintain. Speaking of which, I just realized that we missed one of the arguments: Topology2D optionally takes elemorder (for space filling curve), elempid, and orderindex. Also, h_topology could be Topologies.Topology2D or Topologies.DistributedTopology2D, so those need to optional keyword arguments (we use this in atmos).

• Add some examples on how to use the constructors.

Sounds good, where do you think we should add them?

• Add a NEWS entry that also quickly highlight how to use this (vs the old way)

Sure, I'll add hyperlinks to the constructor and their doc strings.

I think that these functions are very useful and also very user-facing, so I would like to see the docstrings expanded to explain the various terms in a way that is accessible to someone who is not familiar to ClimaCore.

Sounds good.

[charleskawczynski]

My only other thought is: should we put this inside a module? It might be nice if users could do tab completion on ClimaCore.Grids.ConvenienceConstructors.<tab> Otherwise the list will be pretty big and users may not remember all of the names

[charleskawczynski]

I've updated the news, but I'll update it again once the PR is merged so that I can use permanent links

[Sbozzolo]

Thanks for expanding the docstrings.

I don't have strong opinions about putting the constructors in a module. My only thought is that I wouldn't want people to think that they shouldn't use them. To me, the name ConvenienceConstructors has some implication that these are not first-class citizens and I should be using something else.

Can you please also add examples where these are used for something? I don't think that the current examples in the docstrings are enough to tell people how to use them.

It would also be very useful to have a transition guide to distribute to current users. Something like "You are used to doing this, now you can do this".

I encourage you to take this opportunity to also add a page in the documentation describing the computational grids and how to set them up and use them.

[charleskawczynski]

I don't have strong opinions about putting the constructors in a module. My only thought is that I wouldn't want people to think that they shouldn't use them. To me, the name ConvenienceConstructors has some implication that these are not first-class citizens and I should be using something else.

But, they are convenience constructors. All they do is call the other constructors and provide a "flattened constructor" for users, so that they don't have to think about compose-ability. That said, these constructors make (hopefully useful) assumptions about what the user likely wants (e.g., no periodicity in the z-direction).

I don't care too much about the name, I'm also fine with calling the module (assuming we like the idea of putting them into a module) "CommonGrids".

Can you please also add examples where these are used for something? I don't think that the current examples in the docstrings are enough to tell people how to use them.

Sure

It would also be very useful to have a transition guide to distribute to current users. Something like "You are used to doing this, now you can do this".

I'm not sure where this should go, maybe if it's in the module, we could put it an example like this in the module docs? I don't want to put it in individual docs, because there is already a lot of documentation duplication, which I'm not really happy about.

I encourage you to take this opportunity to also add a page in the documentation describing the computational grids and how to set them up and use them.

Let's leave this for a subsequent PR. I don't want to hold people up from being able to use this. I'll open an issue.

[Sbozzolo]

I don't have strong opinions about putting the constructors in a module. My only thought is that I wouldn't want people to think that they shouldn't use them. To me, the name ConvenienceConstructors has some implication that these are not first-class citizens and I should be using something else.

But, they are convenience constructors. All they do is call the other constructors and provide a "flattened constructor" for users, so that they don't have to think about compose-ability. That said, these constructors make (hopefully useful) assumptions about what the user likely wants (e.g., no periodicity in the z-direction).

I don't care too much about the name, I'm also fine with calling the module (assuming we like the idea of putting them into a module) "CommonGrids".

Yes, I agree with you! I just don't want the name to send the wrong message. I like CommonGrids.

I'm not sure where this should go, maybe if it's in the module, we could put it an example like this in the module docs? I don't want to put it in individual docs, because there is already a lot of documentation duplication, which I'm not really happy about.

You could add it in a documentation page and add a @ref to the docstrings. I would also add it to the NEWS so that those who follow the changelog can immediately understand what they need to do.

[charleskawczynski]

Unfortunately, I'm running issues into making this work with different Hypsography options (the default works). In particular, some hypsography constructors accept fields, which means that the space must have already been made. Also, we can't make an artificial one first, since it must match the horizontal grid (which is made inside the convenience constructors). In other words, we'd get an (appropriate) assertion error: AssertionError: Spaces.grid(axes(adaption.surface)) == horizontal_grid. So, I don't think that these convenience constructors can allow sufficient flexibility at the layer of grids. I think we can try to make these convenience constructors for spaces, instead.

I think most of the constructors will be the same, except for the FD ones, which will require Grids.CellCenter / Grids.CellFace as an additional argument.

One downside of this is that users will be required to send "instructions" on how to make the hypsography. It's a bit easier to manage this when it's done incrementally (as we do now with the compose-able interface).

[charleskawczynski]

Alright, the way around this was to allow users to inject a function for computing the hypsography, and this function should basically have access to all of the input arguments.

I know that the documentation can be improved, but my preference would be to iterate on this. I think what this PR has now is an improvement over the main branch.

Can we merge this and iterate on docs?

Next, I'll open a PR that has a thin layer that introduces a CommonSpaces module. Thoughts?

[Sbozzolo]

Thank you! I am fine with starting with this, and we can do another round of documentation when we add the spaces. Critically, what is missing is the link with what people are doing now and we need to explain how people should use these constructors.