search

coecms / xmhw

Xarray version of Marine Heatwaves code by Eric Olivier
https://xmhw.readthedocs.io/en/latest/
Apache License 2.0
21 stars 10 forks source link

add different calendar option #27

Closed paolap closed 2 years ago

paolap commented 3 years ago

Add function to attempt to detect calendar used in input timeseries to make sure correct approach is used in calculation of threshold, currently data is forced to 366 days year, but this wouldn't work with model data using a 360 days calendar.

Thomas-Moore-Creative commented 2 years ago

Hi @paolap, a possibly silly (or obvious) idea but is one option to default to one calendar (or three depending on daily, weekly, and monthly data) and detecting and resampling input data to that / those calendars?

paolap commented 2 years ago

I'm not so keen to add a resample, I think it's better to keep it separate, so a user is fully aware of that happening, and in any case it should be done before starting anyway. Detecting yes, that's what I'm trying to do, to make sure that the handling of the 29Feb data makes sense. So currently (following the original code behaviour) the climatologies are calculated for a 366 days year. Feb 29 is added as a mean of 28 feb. to 1st March values included. So making sure a 360 days works smoothly. I'm also open to weekly and monthly if this use cases make sense, I heard of weekly from one of our students but the use case wasn't really clear to me. Ideally there should be an option where instead of day of th year the threshold is calculated on whatever "time unit" you want to use, then how you define weeks or months is up to you. I'll try to set it up this way, it shouldn't be difficult and it would basically be the same for 360 days case. thanks

paolap commented 2 years ago

@Thomas-Moore-Creative I think I got this working, now there's an option to use the original timestep as base unit for the "day-of-year" coordinate. I tested this with monthly data the climatologies are calculated as monthly values. Then I resampled a daily time series to 5-days intervals and the climatologies have a 5days interval. This also take care of a 360days year case (tested this one too). I'll merge it tomorrow (tilmestep branch) after a few more tests and updating the documentation :-)

paolap commented 2 years ago

I've now updated timestep branch after adding tests and updating the notebook with examples.

paolap commented 2 years ago

This is part of master as of release 0.8.0