-
Notifications
You must be signed in to change notification settings - Fork 5
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
Add remaining water balance terms to basin.arrow #1367
Conversation
Technically these terms can be calculated from flow.arrow and the storage. But since calculating the water balance for a Basin is such a common operation, we include them directly. This also makes it easier to locate water balance errors in time and space, by including the error term. At some point we may also want to add the instantaneous inflow and outflow to the Basin struct, and update them in `formulate_du!`. This would allow us to reject steps with too large of a balance error, using one of the metrics mentioned in #1271. But for now this only adds them to the output via a callback.
@gijsber @DanielTollenaar does this proposal and the change in usage.qmd look ok to you? |
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.
Good stuff
Would it be an idea to integrate the absolute water balance error per basin as a state of the system of differential equations, and throw an error in the same way we do for negative storages if this gets too big? Now that I think of it, why don't we integrate the vertical fluxes this way, just let the solver do it? |
Good stuff! What about considering a How I compute these in Python (think we have to think about something to avoid replace infinite (balance error devided by 0 in- or outflow_rate):
@SouthEndMusic, guess relative_error is something you may use in the solver? |
In 12c77be I renamed @DanielTollenaar I added your @SouthEndMusic I think it is a good idea to let the integrator integrate the flows as well. Do you want to create an issue for it? We should probably do a small test first to get a feeling for it. |
#metric all inflows; agreed, that's better. |
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.
Added some final comments
Co-authored-by: Bart de Koning <[email protected]>
Technically these terms can be calculated from flow.arrow and the storage. But since calculating the water balance for a Basin is such a common operation, we include them directly. This also makes it easier to locate water balance errors in time and space, by including the error term.
At some point we may also want to add the instantaneous inflow and outflow to the Basin struct, and update them in
formulate_du!
. This would allow us to reject steps with too large of a balance error, using one of the metrics mentioned in #1271. But for now this only adds them to the output via a callback.