Skip to content

Conversation

@ltroussellier
Copy link
Contributor

It is just a draft
i need to review it carefully (syntax of everything), especially the DRS etc ..

Does it sound like a good starting point for the guidelines ?

@ltroussellier
Copy link
Contributor Author

#24 for reference

Copy link
Collaborator

@matthew-mizielinski matthew-mizielinski left a comment

Choose a reason for hiding this comment

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

A few minor comments/suggestions, but this looks good to me otherwise

Comment on lines +43 to +45
➡️ [https://github.com/WCRP-CMIP/CMIP7_CVs/](https://github.com/WCRP-CMIP/CMIP7_CVs/)
[https://github.com/WCRP-CMIP/WCRP-Universe/](https://github.com/WCRP-CMIP/WCRP-Universe/)
Guidance documents: [https://wcrp-cmip.github.io/cmip7-guidance/](https://wcrp-cmip.github.io/cmip7-guidance/)
Copy link
Collaborator

Choose a reason for hiding this comment

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

add bullets?

Copy link
Contributor

Choose a reason for hiding this comment

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

Why the link to this guidance repo? (user is already here...)

Should there be a link to the WCRP-Universe without explaining what this is? Might be simpler to just have the link to the CMIP7-CVs repo (otherwise, would esgvoc have to be mentioned/explained?).

Comment on lines 152 to 154
**For further guidance and updates:**
Visit the CMIP7 documentation portal:
🔗 [https://wcrp-cmip.github.io/cmip7-guidance/](https://wcrp-cmip.github.io/cmip7-guidance/)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Would be tempted to drop this.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think the document info is also given at the top? Which seems to me a better place for it.

I don't think we need the link to this guidance repo (?).

@JamesAnstey JamesAnstey changed the title feat : draft for the global attributes guielines feat : draft for the global attributes guidelines Oct 23, 2025
| **Other Required Fields** | `frequency`, `grid_label`, `region`, `realm`, `variable_id`, `tracking_id` | | Define data frequency, grid, domain, and unique file IDs |

**Conditionally Required Attributes:**
Used only when datasets branch from parent simulations (e.g., `branch_time_in_parent`, `parent_activity_id`).
Copy link
Contributor

Choose a reason for hiding this comment

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

"only" --> "mainly" (external_variables is also conditional)

### Notes:
- `timeRangeDD` is omitted for fixed (time-independent) variables.
- Filenames directly encode key experiment and model metadata.

Copy link
Contributor

Choose a reason for hiding this comment

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

Perhaps add sub-bullets under filenames:

    - The first half of the filename, `<variable_id>_<branding_suffix>_<frequency>_<region>`, specifies the reported variable
    - The second half of the filename, `<source_id>_<experiment_id>_<variant_label>[_<timeRangeDD>]`, specifies the model simulation and the time period from it that is included in the file


The `nominal_resolution` describes approximate horizontal grid spacing (e.g., `"1 km"`, `"250 km"`).
It is computed using a standard algorithm (updated from CMIP6 Appendix 2) that ensures consistent comparison of model resolutions across CMIP7 datasets.

Copy link
Contributor

Choose a reason for hiding this comment

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

Should we provide the link given in the EMD:
https://pcmdi.github.io/nominal_resolution/html/index.html
?

Or could just link to the EMD, which specifies how to do it (calculate mean_resolution_km and then use the lookup table in Sec 7.10)

Comment on lines 152 to 154
**For further guidance and updates:**
Visit the CMIP7 documentation portal:
🔗 [https://wcrp-cmip.github.io/cmip7-guidance/](https://wcrp-cmip.github.io/cmip7-guidance/)
Copy link
Contributor

Choose a reason for hiding this comment

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

I think the document info is also given at the top? Which seems to me a better place for it.

I don't think we need the link to this guidance repo (?).

Comment on lines +43 to +45
➡️ [https://github.com/WCRP-CMIP/CMIP7_CVs/](https://github.com/WCRP-CMIP/CMIP7_CVs/)
[https://github.com/WCRP-CMIP/WCRP-Universe/](https://github.com/WCRP-CMIP/WCRP-Universe/)
Guidance documents: [https://wcrp-cmip.github.io/cmip7-guidance/](https://wcrp-cmip.github.io/cmip7-guidance/)
Copy link
Contributor

Choose a reason for hiding this comment

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

Why the link to this guidance repo? (user is already here...)

Should there be a link to the WCRP-Universe without explaining what this is? Might be simpler to just have the link to the CMIP7-CVs repo (otherwise, would esgvoc have to be mentioned/explained?).

@matthew-mizielinski
Copy link
Collaborator

@ltroussellier, this looks reasonably close --I'd like to either merge this in or put in a place holder pointing at the zenodo document (#55) until we can get this in. Have you got the time to look at the comments above today?

@ltroussellier
Copy link
Contributor Author

I'm just starting to get back into the swing of things after the holidays. As for this merge request, everything looks good indeed.

@ltroussellier ltroussellier marked this pull request as ready for review November 4, 2025 11:00
@ltroussellier
Copy link
Contributor Author

i will make the suggested changes as is. I have tried to it quickly but need some care with git interface (im not enough expert to do it quickly)

Co-authored-by: Matthew Mizielinski <[email protected]>
@ltroussellier
Copy link
Contributor Author

did it work ? 😅
feel free to merge it
i will pull the changes after and review it.
I will recreate a Pullrequest if necessary

@matthew-mizielinski
Copy link
Collaborator

@ltroussellier, I've made one tweak to ensure formatting works and I'm going to merge this, but it would be good to address James' comments in a follow-up PR in the near future.

@matthew-mizielinski matthew-mizielinski merged commit 2a391ce into WCRP-CMIP:main Nov 4, 2025
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.

3 participants