-
Notifications
You must be signed in to change notification settings - Fork 6
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
Fastslow initval #190
base: develop
Are you sure you want to change the base?
Fastslow initval #190
Conversation
This is a WIP, @surbhigoel77 ? Can you convert to a draft (link in Reviewers pane). |
Also - have you branched this off the |
Yes, I realised that when I pushed changes to |
No bother - it isn't a big deal 😄 I guess we could |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Definitely the right inputs, but then we need to feed those forward into the calculations. This is a little more finicky than I first thought - I'm going to take it off the list for version 1.0.0 (#206)
init_realised: tuple[NDArray, NDArray, NDArray] | None = None, | ||
) -> None: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Docstring needs updating to include this argument.
if init_realised is not None: | ||
self.init_xi_real, self.init_vcmax_real, self.init_jmax_real = init_realised |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This needs to validate the provided values - mostly just that they are the correct shape. I'm not sure they need to be stored as class attributes.
self.xi_real: NDArray = memory_effect( | ||
self.pmodel_acclim.optchi.xi, alpha=alpha, allow_holdover=allow_holdover |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The init values need to be passed into memory_effect
- they should replace the initial assumption that initial_values: NDArray | None = None
argument to memory_effect
and then the line memory_values[0] = values[0]
can conditionally change to memory_values[0] = initial_values
.
A sensible test for this would be to take the existing test and then split it into two time chunks - feeding the last realised value from the first chunk into the initial values of the second chunk should then duplicate the results of an all in one run.
Description
The
subdaily
module uses an extended form of PModel called theFastSlowPModel
. to estimate two categories of variables - fast variables (that respond to environmental changes instantaneously) and slow variables (that take time to respond to these changes). The module uses amemory_effect
functions that facilitates the calculation of the slow variables as a combination of their lag values and optimal values.In the current implementation of
subdaily
, at the start of the model run, the slow variables take the optimal value as the initial value and update the succeeding values using the:This PR intends to use an approach in which the last available realised value is used as the initial value for$x_{i}$ , $V_{max}$ and $J_{max}$ .
Fixes #82
Type of change
Key checklist
pre-commit
checks:$ pre-commit run -a
$ poetry run pytest
Further checks