search

Deltares / Ribasim

Water resources modeling
https://deltares.github.io/Ribasim/
MIT License
36 stars 5 forks source link

Validate `min_crest_level` (Outlet) and `level` (TabulatedRatingCurve) with upstream `Basin / profile` #1591

Open DanielTollenaar opened 1 week ago

DanielTollenaar commented 1 week ago

What When specifying an Outlet or TabulatedRatingCurve one can specify min_crest_level or level that is lower than the upstream Basin / profile. This is one of the most common causes of model instability/crashes.

Why Validating the min_crest_level and level column with the minimum level upstream Basin / profile prior to modelling would speed-up model debugging.

How Compare min_crest_level and level columns in Outlet or TabulatedRatingCurve with upstream Basin / profile and give a validation error when min_crest_level and level are lower than the minimum level in the basin-profile

visr commented 1 week ago

The min_crest_level is optional, and defaults to -Inf. In that case, or in the case that it is below the Basin bottom, this reduction factor is never applied. If the upstream Basin is about to run dry this reduction factor should still apply, but perhaps can be easily overstepped completely. Do you see these instabilities / crashes also with default adaptive solvers?

We should probably default min_crest_level to the upstream Basin bottom, and validate that it is not set to a value lower than that (but not automatically raise it to the bottom, I want users to be aware of this).

And I think it also makes sense to require TabulatedRatingCurve level to start at or above bottom, just as flow_rate needs to start at 0.

DanielTollenaar commented 1 week ago

Do you see these instabilities / crashes also with default adaptive solvers?

Yes

For the rest, I think we see it the same way. And even if it doesn't/wouldn't serve a numeric purpose, in case of an Outlet of TabulatedRatingCurve (min_crest_)level lower than upstream basin-bottom is (in my view) faulty input that cannot be and therefore should be corrected :-)

Jingru923 commented 4 days ago

Would it be a good idea to set the min_crest_level of Outlet to 0?

gijsber commented 4 days ago

Assuming you want to make 0 the default value.... ... I don't think that is a good idea. 0 is in my opion an arbitrary choice as you do not know at what elevation your model is located. If the majority is below mean sea level and your 0-reference is mean sea level your default value makes life complicated as the modeller might not be aware that you apply this default.