Rename Outlet's min_crest_level to min_upstream_level#1788
Merged
Conversation
Member
|
@visr I've added some migration specific changes here in |
visr
added a commit
that referenced
this pull request
Sep 3, 2024
#1792) Fixes #1771, the remaining part after #1788 was merged. I split out #1789 as a separate issue, though in this PR a lot of the preparatory work is already done (all changes related to TabulatedRatingCurve). This is breaking due to #1791 because of adding new optional columns. It is also breaking that TabulatedRatingCurve and Pump now only support one outflow edge. As far as I know this wasn't used anywhere though. We cannot easily support that anymore since we now look at downstream levels. Users should instead use separate Pumps or TabulatedRatingCurves, which can share the same upstream Basin, but go to different downstream nodes. I decided to start the reduction factors for reaching these limits from 2 centimeters away. There was one existing level based reduction factor, the level difference in the Outlet, which I also adjusted from 10 to 2 centimeter. I feel like 10 centimeter would be too much in areas where the level is controlled up to centimeter accuracies.
visr
added a commit
that referenced
this pull request
Sep 5, 2024
The migration function from #1788 was wrong because I didn't know that Ribasim Python would already add the new optional column by the time it got to the migration. This now drops that one first, to avoid ending up with `min_upstream_level` twice in one dataframe. This also makes the other functions a bit simpler by allowing errors when dropping columns that are no longer needed.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The breaking part of #1771, with added automatic migration.