-
Notifications
You must be signed in to change notification settings - Fork 45
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Clarify the conventions for boundary variables #547
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looking good -- thanks!
ch07.adoc
Outdated
For cells with fewer vertices than the size of vertex dimension, the unneeded elements must appear as the last elements in the vertex dimension and must be assigned the **`_FillValue`**. | ||
CF can currently describe boundaries for cells which have one or two spatial dimensions, but does not provide conventions to describe the boundaries of cells with three spatial dimensions. | ||
Please refer to <<UGRID>> for development of such conventions. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
UGRID only kinda-sorta does 3D -- not sure it should be mentioned here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But it probably should be mentioned somewhere near here ....
Thanks for the PR @JonathanGregory. I will discuss a bit further below. We remove the reasonable assumption and add the no-default statement. It strikes me that we have maybe not concluded on the corner-case when the data producer wants to convey that the cells are in fact points. Should we include the corner-case in the no-default disclaimer?
|
What does "the cells are in fact points" mean? in my mind, points, are well, points, and not cells at all, and that's the usual definition of data in CF that doesn't have cell bounds. the closest I've been able to find for a definition of "cell" is:
"finite" to me means, "not a point". |
I noticed the statement under figs. 7.1 and 7.2 that say: "Tuples I support this pull request. |
@ChrisBarker-NOAA @JonathanGregory I am starting to wonder if our idea of adding a "no-assumption" sentence in Chapter 7 is that good. Re-reading the very start of section 7:
To me, this reads like the absence of The new Maybe the best solution would be to delete the reasonable assumption sentence from Chapter 4, and add a sentence in Section 7 that reads: "In the absence of :bounds, the data represents the point values of a field". Then it is clear what the interpretation is when no :bounds are provided. Sorry, I am going in circles a bit. Removing the sentence from Chapter 4 is still a very good idea. |
I don't think the absence of bounds can imply "point" data unless we make bounds a requirement for data representing cells. In the past, this has not been a requirement, so we can't change this without upsetting backward compatibility. |
In that case, the absence of
|
Is that really the case? how in the world can you have cells if you haven't defined them somehow? If it really was the case that that one could put in data representing cells without defining what the cells are, then I suppose what you had was: These data are on cells of unknown geometry -- seems like a bad idea to me, but if that's what CF used to allow, then I guess it still does. So how do you know if the data are point data or cell data? Is it point data if there is no cell_method, and cell data if there is? in the current text:
OK, so that is a "should" not a must.... But if it IS cell data, then it must (?) have a cell_method and/or a cell_measure (e.g. cell-area). I guess it's not useless to have, e.g. have a cell_method and a cell-area with no defined bounds, though not great. Even a cell_method with no other definition of the cells is still some information. Not sure what this means for the text, but maybe something along the lines of, in the intro to 7, something like: "Data is Representative of Cells if there is a cell_method or a cell metric defined" Along with the "should" for the bounds. |
If you have comments (other than trivial ones on typos and words), please could you make them in #527, not here in the PR. Thanks. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Asside from one micro copy-edit suggestion, this looks good to go!
Thanks for keeping up with the slog!
ch07.adoc
Outdated
|
||
[[cell-boundaries, Section 7.1, "Cell Boundaries"]] | ||
=== Cell Boundaries | ||
|
||
To represent cells we add the attribute **`bounds`** to the appropriate coordinate variable(s). | ||
To represent cells, the **`bounds`** attribute may be added to the appropriate coordinate variable(s). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe "to define cells ..." rather than "represent"? -- no biggie.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks. I've replaced "represent" with "delimit".
ch01.adoc
Outdated
It is often the case that data values are not representative of single points in time, space and other dimensions, but rather of intervals or multidimensional cells. | ||
CF defines a **`bounds`** attribute to specify the extent of intervals or cells. | ||
Because both the <<NUG>> and <<COARDS>> define coordinate variables but not cells or bounds, many applications assume that gridpoints are always located at the centers of their cells. | ||
This assumption does not hold in CF. If bounds are not provided, the location of the gridpoint within the cell is undefined, and nothing can assumed about the location and extent of the cell. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
missing word: and nothing can be assumed...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks. Done.
I support all these modifications and clarifications. |
See issue #527 for discussion of these changes.
Release checklist
cf-conventions.adoc
? Add in two places: on line 3 and under.Additional Authors
inAbout the authors
.cf-conventions.adoc
up to date? Versioning inspired by SemVer.history.adoc
up to date?