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

Validate level with upstream basin #1607

Merged
merged 14 commits into from
Jul 16, 2024
Merged

Conversation

Jingru923
Copy link
Contributor

@Jingru923 Jingru923 commented Jul 3, 2024

Fixes #1591

An error will be given when the level in tabulated rating curve is lower than upstream bottom.

An error will be given when the min_crest_level in outlet is lower than upstream bottom. In the validation function, it will set the min_crest_level to the basin bottom if this parameter was not specified.

@Jingru923 Jingru923 requested a review from visr July 4, 2024 11:00
@Jingru923 Jingru923 marked this pull request as ready for review July 4, 2024 11:03
@Jingru923 Jingru923 requested a review from visr July 5, 2024 08:27
Copy link
Member

@visr visr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks quite close, I have left some comments.

@Jingru923
Copy link
Contributor Author

In the validation of tabulated rating curve level, currently many models that has tabulated rating curve with upstream basin fails. The reason is that in the Ribasim python, the level is [0.0 1.0], but during initialization, we add 1.0 to avoid negative extrapolation for Q (line 121)
image

Once an idea of during validation before the parameter initialization came up, but it is found to be impossible because the validation request the graph information.

For now, I add the 1.0 back during the validation

image

But this make us question the robustness of the extrapolation

Huite opened an issue to research this robustness problem #1621

@Jingru923 Jingru923 requested a review from visr July 11, 2024 14:36
@Jingru923 Jingru923 merged commit 95663da into main Jul 16, 2024
24 of 25 checks passed
@Jingru923 Jingru923 deleted the validate-level-with-upstream-basin branch July 16, 2024 08:31
SouthEndMusic pushed a commit that referenced this pull request Sep 9, 2024
This is like #1607, but now
running the same validation for Pump instead of only Outlet, because
Pump now also supports `min_upstream_level`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Validate min_crest_level (Outlet) and level (TabulatedRatingCurve) with upstream Basin / profile
2 participants