Clarify in docs: UXarray supports only spherical 2D grids with convex faces - #1626
Clarify in docs: UXarray supports only spherical 2D grids with convex faces#1626Sevans711 wants to merge 1 commit into
Conversation
Sevans711
added
bug
Since the PR does only documentation type of work, removed the bug label to avoid any confusions. |
erogluorhan
left a comment
erogluorhan
left a comment
There was a problem hiding this comment.
Thanks a lot for this work! Please find a few inline comments below
| the unstructured part of the grid cannot vary across those dimensions. | ||
| UXarray also assumes that the grid can be represented using UGRID conventions, | ||
| even if it isn't input directly in UGRID format. | ||
| See also: `Supported Models & Grid Formats <user-guide/grid-formats.rst>`_. |
There was a problem hiding this comment.
The rendered version throws 404 for this link.
| ========================== | ||
|
|
||
| Coming soon! | ||
| What does UXarray assume about grids? |
There was a problem hiding this comment.
There might be many more assumptions that are not covered here, so maybe call this "What foundational assumptions does UXarray have about grids and geometries?" or something similar?
| Coming soon! | ||
| What does UXarray assume about grids? | ||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | ||
| UXarray assumes that each grid spans some or all of a 2D spherical surface, and contains only convex faces. |
There was a problem hiding this comment.
What do you mean by this first sentence, global and regional grids?
I think I'd open this first sentence about the most foundational assumption, something like:
"UXarray assumes horizontally unstructured grids with vertical levels, compliant with the UGRID conventions' 3D layered mesh topology, i.e. no fully 3D unstructured topology, either spanning regionally or globally, and consisting only of convex grid faces/cells."
Then, I'd have a 2nd paragraph like:
"UXarray assumes Spherical (latitude/longitude) coordinates by default even though it supports Cartesian (x, y, z) coordinates as well. However, its geometry algorithms assume a spherical surface - unit sphere by default."
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | ||
| UXarray assumes that each grid spans some or all of a 2D spherical surface, and contains only convex faces. | ||
| While other dimensions such as time or elevation can be represented easily enough, | ||
| the unstructured part of the grid cannot vary across those dimensions. |
There was a problem hiding this comment.
This sentence is a bit hard to read, and I think it could be dropped if you accept the previous one.
| While other dimensions such as time or elevation can be represented easily enough, | ||
| the unstructured part of the grid cannot vary across those dimensions. | ||
| UXarray also assumes that the grid can be represented using UGRID conventions, | ||
| even if it isn't input directly in UGRID format. |
There was a problem hiding this comment.
Similarly, this phrase could be dropped if you accept the previous one.
| UXarray focuses on 2D unstructured grids which lie entirely on the surface of a sphere, | ||
| and contain only convex faces. Data spanning extra dimensions such as time or elevation | ||
| can be represented easily enough, as long as the grid does not vary across them. | ||
| Other types of grids are not supported. |
There was a problem hiding this comment.
Please rephrase accordingly with the suggestions above
| Additional dimensions such as time or elevation can be represented easily enough | ||
| by including corresponding extra dimensions on the data variables. | ||
| However, the faces, nodes, and edges of each unstructured grid must lie entirely | ||
| on the surface of a sphere. |
There was a problem hiding this comment.
Please rephrase accordingly with the suggestions above
2 participants
Closes #1597
Closes #1615
Overview
Clarifies in readthedocs pages: UXarray supports only spherical 2D grids with convex faces.
As per discussion in issue 1615:
As per discussion in issue 1597:
.. note::clarifying these assumptions in the "Why UXarray?" overview page.Labeling this PR with "bug" in addition to "documentation", as in 1615, because uxarray currently silently handles non-convex cases incorrectly.
This PR does not update the UXarray README as originally proposed, because these details might be too specific to include in the README itself. But it should still solve the fundamental problem of lack of clarity in the docs regarding these assumptions, so it should be safe to close the underlying issues when this PR merges.
PR Checklist
General
Documentation
_) and have been added todocs/internal_api/index.rstdocs/user_api/index.rst