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

convert_calendar could convert day-of-year data #1283

Closed
2 tasks done
aulemahal opened this issue Jan 26, 2023 · 0 comments · Fixed by #1406
Closed
2 tasks done

convert_calendar could convert day-of-year data #1283

aulemahal opened this issue Jan 26, 2023 · 0 comments · Fixed by #1406
Assignees
Labels
enhancement New feature or request
Milestone

Comments

@aulemahal
Copy link
Collaborator

Addressing a Problem?

Some indicators return day-of-year data. The values are thus constrained by the calendar of the input. A 360_day timeseries will have a maximal value of 360. When mixing data from different calendar, like when doing ensemble statistics, this introduces a bias in the statistics. For example, if all members agree that this event happens on the last day of the year, some will have 360, some will have 365 and some 366. The mean will be lower than 365, and the standard deviation will be non-zero, even though every model agrees.

Potential Solution

convert_calendar could detect day-of-year data through the is_dayofyear attribute that xclim adds on such cases.

The basic conversion could be : doy_out = out.time.dt.days_in_year * doy_in / doy_in.time.dt.day_in_year.

Additional context

Caveats:

  1. This introduces floating point day of year, which might not be fully expected in some cases.
  2. For 360_day calendar, this is equivalent to the align_on='year' timestamp conversion method. What about the align_on='date' ? That would need a mapping of doy values, not so complex, just a bit heavier. And should the user be able to decide on which method to use separately on the time coordinate and on the data?
  3. is_dayofyear is xclim-specific. There is no CF standards about this, AFAIK. This would not be portable to xarray's convert_calendar.

Contribution

  • I would be willing/able to open a Pull Request to contribute this feature.

Code of Conduct

  • I agree to follow this project's Code of Conduct
@aulemahal aulemahal added the enhancement New feature or request label Jan 26, 2023
@aulemahal aulemahal added this to the v0.42 milestone Jan 26, 2023
@Zeitsperre Zeitsperre modified the milestones: v0.42, Summer 2023, v0.43 Mar 21, 2023
@Zeitsperre Zeitsperre modified the milestones: v0.43, v0.44 May 8, 2023
@Zeitsperre Zeitsperre modified the milestones: v0.44, Summer 2023 Jun 13, 2023
@aulemahal aulemahal mentioned this issue Jun 28, 2023
5 tasks
aulemahal added a commit that referenced this issue Jun 29, 2023
<!--Please ensure the PR fulfills the following requirements! -->
<!-- If this is your first PR, make sure to add your details to the
AUTHORS.rst! -->
### Pull Request Checklist:
- [x] This PR addresses an already opened issue (for bug fixes /
features)
    - This PR fixes #1283 
- [x] Tests for the changes have been added (for bug fixes / features)
- [x] (If applicable) Documentation has been added / updated (for bug
fixes / features)
- [x] CHANGES.rst has been updated (with summary of main changes)
- [x] Link to issue (:issue:`number`) and pull request (:pull:`number`)
has been added

### What kind of change does this PR introduce?

* New `convert_doy` function to convert the calendar reference of doy
data. Two options :
- "year" : doy is seen as the fraction of the year. I.e. : 360 in
`360_day` converts to 365 in `noleap`.
- "date" : doy is seen as a date (MM-DD). 360 in `360_day` converts to
364 in `noleap` (December 30th).
* Convert calendar can automatically apply `convert_doy` if its new
argument `doy=True` (or `doy='date'`).
* `convert_doy` will NOT convert the time coordinate.

### Does this PR introduce a breaking change?
No.

### Other information:
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants