ImperialCollegeLondon / pyrealm

Development of the pyrealm package, providing an integrated toolbox for modelling plant productivity, growth and demography using Python.
https://pyrealm.readthedocs.io/
MIT License
19 stars 8 forks source link

Fix time handling in SPLASH #165

Closed davidorme closed 5 months ago

davidorme commented 7 months ago

At the moment, SPLASH assumes a fixed year length for SplashModel.estimate_initial_soil_moisture, but this needs to shift to a calendar aware implementation.

Although the original SPLASH v1 used 1st January as the year start in example, I don't think we care about the first day of the time series, just that we have a year's worth of data to equilibrate.

The code to tackle starts here:

https://github.com/ImperialCollegeLondon/pyrealm/blob/8c8698513982d985ea215d5fbc575958167ccd0c/pyrealm/splash/splash.py#L147-L164

tztsai commented 7 months ago

Is it okay to just loop over the whole calendar object, like for day_idx, _ in enumerate(self.tc)?

davidorme commented 7 months ago

What this ideally needs to do is to ensure, given a starting point of YYYY-MM-DD, that YYYY+1-MM-DD exists in the inputs, and then this method can iterate up to that point in the dataset. For leap years, that simply has an extra day in the loop. The difficult case is if the first date is YYYY-02-29.

tztsai commented 7 months ago

Does it have to end on YYYY+1-MM-DD? If not, maybe we can just iterate over 366 days.

davidorme commented 7 months ago

That's sort of where we are now 😄

The underlying idea is that this loop over days is repeated until the estimated soil moisture for YYYY-MM-DD and YYYY+1-MM-DD converge. I think that should always be until the matching day - although the difference when using a fixed number of days is likely to be small. Starting on 29th February should probably just map to 28th February the following year.