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

[Feature]: More Sophisticated Bounds Handling in Temporal Averaging Operations #594

Open
pochedls opened this issue Jan 31, 2024 · 1 comment
Labels
type: enhancement New enhancement request

Comments

@pochedls
Copy link
Collaborator

pochedls commented Jan 31, 2024

Is your feature request related to a problem?

xcdat temporal averaging operations currently bin data by the labelled time point with the weights derived from the difference in the time bounds. This works for most conventional climate data: a timepoint of 2020-01-16 12:00 with bounds of [2020-01-01 00:00, 2020-02-01 00:00] would be given 31 days of weight in January (e.g., in creating an annual average or climatology), which is correct.

There are reasonable instances where this wouldn't work. Imagine pentad data with a time point of 2020-02-02 12:00 with bounds of [2020-01-31 00:00, 2020-02-05 00:00]. This time point should be given one day of weight in January and four days of weight in February. The current algorithm (e.g., for monthly averaging) assigns all five days of weight in February (the labelled time point).

Describe the solution you'd like

Weights should be assigned based on the time period that they fall into. This would mean that a given time point can contribute to averages in more than one time interval

Describe alternatives you've considered

Solutions for the time being would be to update documentation to note that:

  • Weights are determined from the difference in the bounds
  • Data is grouped into the labelled time point for averaging operations
  • If one time point spans across the time intervals that you are averaging into, then weights are not properly assigned
  • get_time_bounds generally assumes data with frequencies of annual, monthly, daily, or sub-daily (I thought of this while writing this issue)
  • Perhaps other disclaimers I haven't thought of

Additional context

I'm not sure cdms / cdutil covers this case; it would be helpful to determine what cdutil does.

This seems like it could be challenging issue to address in general and might require a major refactor of the logic use for existing temporal averaging calculations.

@tomvothecoder
Copy link
Collaborator

I opened up PR #601 to implement the documentation updates suggested in your alternative solution.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: enhancement New enhancement request
Projects
Status: Todo
Development

No branches or pull requests

2 participants