Skip to content
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

Reference UGRID conventions in CF #459

Merged
merged 56 commits into from Oct 23, 2023

Conversation

davidhassell
Copy link
Contributor

@davidhassell davidhassell commented Sep 21, 2023

See issue #153 or discussion of these changes.

Note that #153 also depends on https://github.com/cf-convention/cf-convention.github.io/pull/210/files

This PR replaces #353

Release checklist

  • Authors updated in cf-conventions.adoc?
  • Next version in cf-conventions.adoc up to date? Versioning inspired by SemVer.
  • history.adoc up to date?
  • Conformance document up-to-date?

For maintainers

After the merge remember to delete the source branch.
Tags are set at the conclusion of the annual meeting; until then master always is a draft for the next version.

@davidhassell davidhassell linked an issue Sep 22, 2023 that may be closed by this pull request
@ChrisBarker-NOAA
Copy link
Contributor

I can't say I carefully proofread it, but it looks good to me!

Thanks for all the hard work.

Copy link
Member

@sadielbartholomew sadielbartholomew left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This all looks sensible overall to me. I have raised a small number of questions and thoughts in-line, which relate more to the general underlying Issue #153 than the PR itself, but it feels useful to point to specific points from the new text suggested here for context.

Regarding the new data model:

  • The update to cfdm_cf_concepts.svg, extending with the two new variables, seems very natural to me.
  • As for the main CF Data Model schematic (here in images/cfdm_field.svg), it took a while for me to get my head around the 'new way', but now I think that it makes sense conceptually.
    • Concerning specifically the change in the schematic, I thought it could be useful to have some summary of how the data model has changed, because the repositioning of nodes and rearrangement of relationship lines in the diagram could easily make it seem to someone who doesn't take a detailed look that it has changed quite drastically, but all that has really changed is that we have the new constructs 'DomainTopology' and 'CellConnectivity', right?

A summary indicating how UGRID relates to other parts of the CF conventions, and which features of UGRID are excluded from CF, can be found in <<mesh-topology-variables>>.
To reduce the chance of ambiguities arising from their accidental re-use, all of the UGRID standardized attributes are specified in <<appendix-mesh-topology-attributes>> and <<attribute-appendix>>.

The UGRID conventions have their own conformance document, which should be used in conjunction with the CF conformance document when checking the validity of datasets.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On this note, I am wondering: how do we expect, if at all, that compliance checker tools such as the CF Checker can verify UGRID-related aspects? Will anything in the CF Conformance Document need updating or amending due to the coupling with UGRID and CF?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The conformance document has been updated: https://github.com/cf-convention/cf-conventions/pull/459/files#diff-3b9c470edad8a09f463987db632803f1ecc22561199fa5771745ad472a62e0ee - which just devolves conformance to the UGRID conformance document.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

to have some summary of how the data model has changed,

I'm not in favour of writing into the conventions how they are different to a certain previous versions. That is the role of the issue discussion, I think.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The conformance document has been updated

Aha, sorry I clearly completely missed that when working through the PR 🙂 Devolving sounds wise. All good!

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

RE:

I'm not in favour of writing into the conventions how they are different to a certain previous versions. That is the role of the issue discussion, I think.

Fair enough, that seems sensible - but for the record and to check my understanding, since we are linked to the Issue here - is it indeed what I understand from looking in detail, that:

all that has really changed is that we have the new constructs 'DomainTopology' and 'CellConnectivity'

?

ch05.adoc Show resolved Hide resolved
ch01.adoc Show resolved Hide resolved
@ChrisBarker-NOAA
Copy link
Contributor

This is ready to go -- thanks all for the contributions!

@ChrisBarker-NOAA ChrisBarker-NOAA merged commit 509a206 into cf-convention:main Oct 23, 2023
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Reference UGRID conventions in CF
3 participants