feat(netcdf output)!: Produce netCDF-files that follow CF conventions - #549
feat(netcdf output)!: Produce netCDF-files that follow CF conventions#549mpvginde wants to merge 5 commits into
Conversation
gmertes
changed the title
gmertes
left a comment
gmertes
left a comment
There was a problem hiding this comment.
Thanks for this! I'll defer to others with more knowledge of CF conventions to review that part. We may also want to present this at ATS just for visibility, there may be other users relying on the current behaviour.
|
Note to maintainers: let's do a release before merging. |
| if reference_date := getattr(self.context, "reference_date", None): | ||
| self.reference_date = reference_date | ||
| # start date of the forecast | ||
| reference_date = getattr(self.context, "reference_date", None) or state["date"] |
There was a problem hiding this comment.
@gmertes , I guess here the wrong reference_date can be set if the context doesn't contain reference_date (when does this happen?) and write_initial_state=False. I guess in that case state["date"] contains the valid time of the first timestep?
There was a problem hiding this comment.
More tech debt 🥰
The getattr can be replaced with a simple self.context.reference_date: after the recent Runner refactor, the context (runner) will always have the reference_date attribute (see here), but it can be None.
It will be None when no date is set in the config, and then we rely on the input to find the date in the input file. The input will attach the date it found to the state, and indeed that's where we then find it.
The logic is messy and could be improved, but I don't think there is a scenario where the reference date can be wrong or diverge.
There was a problem hiding this comment.
Ok, makes sense now. See some thoughts on improving here: #551
aaron-hopkinson
left a comment
aaron-hopkinson
left a comment
There was a problem hiding this comment.
A few minor changes (typos).
I think we need to include a global attribute that states the version of the conventions that the data adheres to, see:
https://cfconventions.org/Data/cf-conventions/cf-conventions-1.13/cf-conventions.html#identification-of-conventions
| self.reference_date_var.standard_name = "forecast_reference_time" | ||
| self.reference_date_var.long_name = "start time of forecast" | ||
| self.reference_date_var.units = f"seconds since {EPOCH}" | ||
| self.reference_date_var.calender = CALENDAR |
There was a problem hiding this comment.
Typo
| self.reference_date_var.calender = CALENDAR | |
| self.reference_date_var.calendar = CALENDAR |
| self.time_var.axis = "T" | ||
|
|
||
| # forecast period / lead time | ||
| # time dimension auxilary coordinate |
There was a problem hiding this comment.
| # time dimension auxilary coordinate | |
| # time dimension auxiliary coordinate |
| self.period_var.units = "seconds" | ||
|
|
||
| # latitude / longitude | ||
| # values dimension auxilary coordinates |
There was a problem hiding this comment.
| # values dimension auxilary coordinates | |
| # values dimension auxiliary coordinates |
github-project-automation
Bot
moved this from Now In Progress
to Under Review
in Anemoi-dev
|
@mpvginde Just out of interest, have you tried passing data outputted from this change through https://github.com/cedadev/cf-checker? That would be a good way of checking for CF compliance. |
4 participants
Currently the netCDF output produced by inference is quite minimal in terms of coordinate metadata.
Some issues:
write_initial_state: false.xr.open_mfdatasets()This PR updates the netCDF writing, bringing the netCDF-output in line with CF conventions. Which in term provides better
xarray-handlingAs a contributor to the Anemoi framework, please ensure that your changes include unit tests, updates to any affected dependencies and documentation, and have been tested in a parallel setting (i.e., with multiple GPUs). As a reviewer, you are also responsible for verifying these aspects and requesting changes if they are not adequately addressed. For guidelines about those please refer to https://anemoi.readthedocs.io/en/latest/
By opening this pull request, I affirm that all authors agree to the Contributor License Agreement.