Rename Outlet's min_crest_level to min_upstream_level (#1788)

The breaking part of #1771,
with added automatic migration.

---------

Co-authored-by: Maarten Pronk <[email protected]>

core/src/parameter.jl

@@ -519,7 +519,7 @@ continuous_control_type: one of None, ContinuousControl, PidControl
     flow_rate::Cache = cache(length(node_id))
     min_flow_rate::Vector{Float64} = zeros(length(node_id))
     max_flow_rate::Vector{Float64} = fill(Inf, length(node_id))
-    min_crest_level::Vector{Float64} = fill(-Inf, length(node_id))
+    min_upstream_level::Vector{Float64} = fill(-Inf, length(node_id))
     control_mapping::Dict{Tuple{NodeID, String}, ControlStateUpdate} = Dict()
     continuous_control_type::Vector{ContinuousControlType.T} =
         fill(ContinuousControlType.None, length(node_id))
@@ -532,7 +532,7 @@ continuous_control_type: one of None, ContinuousControl, PidControl
         flow_rate,
         min_flow_rate,
         max_flow_rate,
-        min_crest_level,
+        min_upstream_level,
         control_mapping,
         continuous_control_type,
     )
@@ -545,7 +545,7 @@ continuous_control_type: one of None, ContinuousControl, PidControl
                 flow_rate,
                 min_flow_rate,
                 max_flow_rate,
-                min_crest_level,
+                min_upstream_level,
                 control_mapping,
                 continuous_control_type,
             )

core/src/read.jl

@@ -491,8 +491,12 @@ end
 
 function Outlet(db::DB, config::Config, graph::MetaGraph)::Outlet
     static = load_structvector(db, config, OutletStaticV1)
-    defaults =
-        (; min_flow_rate = 0.0, max_flow_rate = Inf, min_crest_level = -Inf, active = true)
+    defaults = (;
+        min_flow_rate = 0.0,
+        max_flow_rate = Inf,
+        min_upstream_level = -Inf,
+        active = true,
+    )
     parsed_parameters, valid = parse_static_and_time(db, config, Outlet; static, defaults)
 
     if !valid
@@ -518,7 +522,7 @@ function Outlet(db::DB, config::Config, graph::MetaGraph)::Outlet
         flow_rate,
         parsed_parameters.min_flow_rate,
         parsed_parameters.max_flow_rate,
-        parsed_parameters.min_crest_level,
+        parsed_parameters.min_upstream_level,
         parsed_parameters.control_mapping,
     )
 end

core/src/schema.jl

@@ -86,7 +86,7 @@ end
     flow_rate::Float64
     min_flow_rate::Union{Missing, Float64}
     max_flow_rate::Union{Missing, Float64}
-    min_crest_level::Union{Missing, Float64}
+    min_upstream_level::Union{Missing, Float64}
     control_state::Union{Missing, String}
 end
 

core/src/solve.jl

@@ -554,7 +554,7 @@ function formulate_flow!(
         min_flow_rate,
         max_flow_rate,
         continuous_control_type,
-        min_crest_level,
+        min_upstream_level,
     ) in zip(
         outlet.node_id,
         outlet.inflow_edge,
@@ -564,7 +564,7 @@ function formulate_flow!(
         outlet.min_flow_rate,
         outlet.max_flow_rate,
         outlet.continuous_control_type,
-        outlet.min_crest_level,
+        outlet.min_upstream_level,
     )
         if !(active || all_nodes_active) ||
            (continuous_control_type != continuous_control_type_)
@@ -588,7 +588,7 @@ function formulate_flow!(
 
         # No flow out outlet if source level is lower than minimum crest level
         if src_level !== nothing
-            q *= reduction_factor(src_level - min_crest_level, 0.1)
+            q *= reduction_factor(src_level - min_upstream_level, 0.1)
         end
 
         q = clamp(q, min_flow_rate, max_flow_rate)

core/src/validation.jl

@@ -362,12 +362,12 @@ Validate Outlet crest level and fill in default values
 """
 function valid_outlet_crest_level!(graph::MetaGraph, outlet::Outlet, basin::Basin)::Bool
     errors = false
-    for (id, crest) in zip(outlet.node_id, outlet.min_crest_level)
+    for (id, crest) in zip(outlet.node_id, outlet.min_upstream_level)
         id_in = inflow_id(graph, id)
         if id_in.type == NodeType.Basin
             basin_bottom_level = basin_bottom(basin, id_in)[2]
             if crest == -Inf
-                outlet.min_crest_level[id.idx] = basin_bottom_level
+                outlet.min_upstream_level[id.idx] = basin_bottom_level
             elseif crest < basin_bottom_level
                 @error "Minimum crest level of $id is lower than bottom of upstream $id_in" crest basin_bottom_level
                 errors = true

core/test/run_models_test.jl

@@ -372,11 +372,11 @@ end
     outlet_flow =
         filter([:from_node_id, :to_node_id] => (from, to) -> from == 2 && to == 3, flow)
 
-    t_min_crest_level =
-        level.t[2] * (outlet.min_crest_level[1] - level.u[1]) / (level.u[2] - level.u[1])
+    t_min_upstream_level =
+        level.t[2] * (outlet.min_upstream_level[1] - level.u[1]) / (level.u[2] - level.u[1])
 
     # No outlet flow when upstream level is below minimum crest level
-    @test all(@. outlet_flow.flow_rate[t <= t_min_crest_level] == 0)
+    @test all(@. outlet_flow.flow_rate[t <= t_min_upstream_level] == 0)
 
     t = Ribasim.tsaves(model)
     t_maximum_level = level.t[2]

core/test/validation_test.jl

@@ -418,7 +418,7 @@ end
     parameters = model.integrator.p
 
     (; graph, outlet, basin) = parameters
-    outlet.min_crest_level[1] = invalid_level
+    outlet.min_upstream_level[1] = invalid_level
 
     logger = TestLogger()
     with_logger(logger) do

docs/guide/examples.ipynb

@@ -1804,23 +1804,23 @@
     "# Set up outlet\n",
     "model.outlet.add(\n",
     "    Node(2, Point(0.0, -1.0)),\n",
-    "    [outlet.Static(flow_rate=[2 * 0.5 / 3600], min_crest_level=[0.0])],\n",
+    "    [outlet.Static(flow_rate=[2 * 0.5 / 3600], min_upstream_level=[0.0])],\n",
     ")\n",
     "model.outlet.add(\n",
     "    Node(5, Point(0.0, -3.0)),\n",
-    "    [outlet.Static(flow_rate=[0.5 / 3600], min_crest_level=[1.95])],\n",
+    "    [outlet.Static(flow_rate=[0.5 / 3600], min_upstream_level=[1.95])],\n",
     ")\n",
     "model.outlet.add(\n",
     "    Node(7, Point(1.0, -4.0)),\n",
-    "    [outlet.Static(flow_rate=[0.5 / 3600], min_crest_level=[1.45])],\n",
+    "    [outlet.Static(flow_rate=[0.5 / 3600], min_upstream_level=[1.45])],\n",
     ")\n",
     "model.outlet.add(\n",
     "    Node(9, Point(3.0, -4.0)),\n",
-    "    [outlet.Static(flow_rate=[0.5 / 3600], min_crest_level=[0.95])],\n",
+    "    [outlet.Static(flow_rate=[0.5 / 3600], min_upstream_level=[0.95])],\n",
     ")\n",
     "model.outlet.add(\n",
     "    Node(11, Point(4.0, -3.0)),\n",
-    "    [outlet.Static(flow_rate=[0.5 / 3600], min_crest_level=[0.45])],\n",
+    "    [outlet.Static(flow_rate=[0.5 / 3600], min_upstream_level=[0.45])],\n",
     ")"
    ]
   },

docs/reference/node/outlet.qmd

@@ -22,7 +22,7 @@ active                | Bool    | -            | (optional, default true)
 flow_rate             | Float64 | $m^3 s^{-1}$ | non-negative
 min_flow_rate         | Float64 | $m^3 s^{-1}$ | (optional, default 0.0)
 max_flow_rate         | Float64 | $m^3 s^{-1}$ | (optional)
-min_crest_level       | Float64 | $m$          | (optional)
+min_upstream_level       | Float64 | $m$          | (optional)
 
 # Equations
 

python/ribasim/ribasim/__init__.py

@@ -1,5 +1,5 @@
 __version__ = "2024.10.0"
-__schema_version__ = 1
+__schema_version__ = 2
 
 from ribasim.config import Allocation, Logging, Node, Solver
 from ribasim.geometry.edge import EdgeTable

python/ribasim/ribasim/input_base.py

@@ -177,7 +177,7 @@ def _check_schema(cls, v: DataFrame[TableT]):
             if db_path is not None:
                 version = _get_db_schema_version(db_path)
                 if version < ribasim.__schema_version__:
-                    v = cls.tableschema().migrate(v)
+                    v = cls.tableschema().migrate(v, version)
             for colname in v.columns:
                 if colname not in cls.columns() and not colname.startswith("meta_"):
                     raise ValueError(

python/ribasim/ribasim/migrations.py

@@ -3,69 +3,84 @@
 from geopandas import GeoDataFrame
 from pandas import DataFrame
 
+# On each breaking change, increment the __schema_version__ by one.
+# Do the same for write_schema_version in ribasim_qgis/core/geopackage.py
 
-def nodeschema_migration(gdf: GeoDataFrame) -> GeoDataFrame:
-    if "node_id" in gdf.columns:
+
+def nodeschema_migration(gdf: GeoDataFrame, schema_version: int) -> GeoDataFrame:
+    if "node_id" in gdf.columns and schema_version == 0:
         warnings.warn("Migrating outdated Node table.", UserWarning)
         assert gdf["node_id"].is_unique, "Node IDs have to be unique."
         gdf.set_index("node_id", inplace=True)
 
     return gdf
 
 
-def edgeschema_migration(gdf: GeoDataFrame) -> GeoDataFrame:
-    if "from_node_type" in gdf.columns:
+def edgeschema_migration(gdf: GeoDataFrame, schema_version: int) -> GeoDataFrame:
+    if "from_node_type" in gdf.columns and schema_version == 0:
         warnings.warn("Migrating outdated Edge table.", UserWarning)
         gdf.drop("from_node_type", inplace=True, axis=1)
-    if "to_node_type" in gdf.columns:
+    if "to_node_type" in gdf.columns and schema_version == 0:
         warnings.warn("Migrating outdated Edge table.", UserWarning)
         gdf.drop("to_node_type", inplace=True, axis=1)
-    if "edge_id" in gdf.columns:
+    if "edge_id" in gdf.columns and schema_version == 0:
         warnings.warn("Migrating outdated Edge table.", UserWarning)
         gdf.set_index("edge_id", inplace=True)
 
     return gdf
 
 
-def basinstaticschema_migration(df: DataFrame) -> DataFrame:
-    if "urban_runoff" in df.columns:
-        warnings.warn("Migrating outdated Basin / Static table.", UserWarning)
+def basinstaticschema_migration(df: DataFrame, schema_version: int) -> DataFrame:
+    if "urban_runoff" in df.columns and schema_version == 0:
+        warnings.warn("Migrating outdated Basin / static table.", UserWarning)
         df.drop("urban_runoff", inplace=True, axis=1)
 
     return df
 
 
-def basintimeschema_migration(df: DataFrame) -> DataFrame:
-    if "urban_runoff" in df.columns:
-        warnings.warn("Migrating outdated Basin / Time table.", UserWarning)
+def basintimeschema_migration(df: DataFrame, schema_version: int) -> DataFrame:
+    if "urban_runoff" in df.columns and schema_version == 0:
+        warnings.warn("Migrating outdated Basin / time table.", UserWarning)
         df.drop("urban_runoff", inplace=True, axis=1)
 
     return df
 
 
-def continuouscontrolvariableschema_migration(df: DataFrame) -> DataFrame:
-    if "listen_node_type" in df.columns:
+def continuouscontrolvariableschema_migration(
+    df: DataFrame, schema_version: int
+) -> DataFrame:
+    if "listen_node_type" in df.columns and schema_version == 0:
         warnings.warn(
-            "Migrating outdated ContinuousControl / Variable table.", UserWarning
+            "Migrating outdated ContinuousControl / variable table.", UserWarning
         )
         df.drop("listen_node_type", inplace=True, axis=1)
 
     return df
 
 
-def discretecontrolvariableschema_migration(df: DataFrame) -> DataFrame:
-    if "listen_node_type" in df.columns:
+def discretecontrolvariableschema_migration(
+    df: DataFrame, schema_version: int
+) -> DataFrame:
+    if "listen_node_type" in df.columns and schema_version == 0:
         warnings.warn(
-            "Migrating outdated DiscreteControl / Variable table.", UserWarning
+            "Migrating outdated DiscreteControl / variable table.", UserWarning
         )
         df.drop("listen_node_type", inplace=True, axis=1)
 
     return df
 
 
-def pidcontrolstaticschema_migration(df: DataFrame) -> DataFrame:
-    if "listen_node_type" in df.columns:
-        warnings.warn("Migrating outdated PidControl / Static table.", UserWarning)
+def pidcontrolstaticschema_migration(df: DataFrame, schema_version: int) -> DataFrame:
+    if "listen_node_type" in df.columns and schema_version == 0:
+        warnings.warn("Migrating outdated PidControl / static table.", UserWarning)
         df.drop("listen_node_type", inplace=True, axis=1)
 
     return df
+
+
+def outletstaticschema_migration(df: DataFrame, schema_version: int) -> DataFrame:
+    if schema_version == 1:
+        warnings.warn("Migrating outdated Outlet / static table.", UserWarning)
+        df.rename(columns={"min_crest_level": "min_upstream_level"}, inplace=True)
+
+    return df

python/ribasim/ribasim/schemas.py

@@ -19,11 +19,11 @@ def _index_name(self) -> str:
         return "fid"
 
     @classmethod
-    def migrate(cls, df: Any) -> Any:
-        f: Callable[[Any], Any] = getattr(
-            migrations, str(cls.__name__).lower() + "_migration", lambda x: x
+    def migrate(cls, df: Any, schema_version: int) -> Any:
+        f: Callable[[Any, Any], Any] = getattr(
+            migrations, str(cls.__name__).lower() + "_migration", lambda x, _: x
         )
-        return f(df)
+        return f(df, schema_version)
 
 
 class BasinConcentrationExternalSchema(_BaseSchema):
@@ -234,7 +234,7 @@ class OutletStaticSchema(_BaseSchema):
     flow_rate: Series[float] = pa.Field(nullable=False)
     min_flow_rate: Series[float] = pa.Field(nullable=True)
     max_flow_rate: Series[float] = pa.Field(nullable=True)
-    min_crest_level: Series[float] = pa.Field(nullable=True)
+    min_upstream_level: Series[float] = pa.Field(nullable=True)
     control_state: Series[str] = pa.Field(nullable=True)
 
 

python/ribasim/tests/test_schemas.py

@@ -1,6 +1,7 @@
 from unittest.mock import patch
 
 import pytest
+import ribasim
 from pydantic import ValidationError
 from ribasim import Model
 from ribasim.db_utils import _get_db_schema_version, _set_db_schema_version
@@ -34,9 +35,10 @@ def test_model_schema(basic, tmp_path):
     toml_path = tmp_path / "basic.toml"
     db_path = tmp_path / "database.gpkg"
     basic.write(toml_path)
-    assert _get_db_schema_version(db_path) == 1
-    _set_db_schema_version(db_path, 2)
-    assert _get_db_schema_version(db_path) == 2
+
+    assert _get_db_schema_version(db_path) == ribasim.__schema_version__
+    _set_db_schema_version(db_path, 0)
+    assert _get_db_schema_version(db_path) == 0
 
 
 def test_geometry_validation():

python/ribasim_testmodels/ribasim_testmodels/basic.py

@@ -381,7 +381,7 @@ def outlet_model():
     # Setup the outlet
     model.outlet.add(
         Node(2, Point(1.0, 0.0)),
-        [outlet.Static(flow_rate=[1e-3], min_crest_level=[2.0])],
+        [outlet.Static(flow_rate=[1e-3], min_upstream_level=[2.0])],
     )
 
     # Setup the edges

python/ribasim_testmodels/ribasim_testmodels/doc_example.py

@@ -134,23 +134,23 @@ def local_pidcontrolled_cascade_model():
     # Set up outlet
     model.outlet.add(
         Node(2, Point(0.0, -1.0)),
-        [outlet.Static(flow_rate=[4 * 0.5 / 3600], min_crest_level=[0.0])],
+        [outlet.Static(flow_rate=[4 * 0.5 / 3600], min_upstream_level=[0.0])],
     )
     model.outlet.add(
         Node(5, Point(0.0, -3.0)),
-        [outlet.Static(flow_rate=[0.5 / 3600], min_crest_level=[1.95])],
+        [outlet.Static(flow_rate=[0.5 / 3600], min_upstream_level=[1.95])],
     )
     model.outlet.add(
         Node(7, Point(1.0, -4.0)),
-        [outlet.Static(flow_rate=[4 * 0.5 / 3600], min_crest_level=[1.45])],
+        [outlet.Static(flow_rate=[4 * 0.5 / 3600], min_upstream_level=[1.45])],
     )
     model.outlet.add(
         Node(9, Point(3.0, -4.0)),
-        [outlet.Static(flow_rate=[0.5 / 3600], min_crest_level=[0.95])],
+        [outlet.Static(flow_rate=[0.5 / 3600], min_upstream_level=[0.95])],
     )
     model.outlet.add(
         Node(11, Point(4.0, -3.0)),
-        [outlet.Static(flow_rate=[0.5 / 3600], min_crest_level=[0.45])],
+        [outlet.Static(flow_rate=[0.5 / 3600], min_upstream_level=[0.45])],
     )
 
     model.edge.add(model.basin[1], model.outlet[2])

ribasim_qgis/core/geopackage.py

@@ -51,7 +51,7 @@ def layers(path: Path) -> list[str]:
     return layers
 
 
-def write_schema_version(path: Path, version: int = 1) -> None:
+def write_schema_version(path: Path, version: int = 2) -> None:
     """Write the schema version to the geopackage."""
     with sqlite3_cursor(path) as cursor:
         cursor.execute(

ribasim_qgis/core/nodes.py

@@ -570,7 +570,7 @@ def attributes(cls) -> list[QgsField]:
             QgsField("flow_rate", QVariant.Double),
             QgsField("min_flow_rate", QVariant.Double),
             QgsField("max_flow_rate", QVariant.Double),
-            QgsField("min_crest_level", QVariant.Double),
+            QgsField("min_upstream_level", QVariant.Double),
             QgsField("control_state", QVariant.String),
         ]