From 2041ddfb7f21e847153a305a0214e6c30c4a6d4a Mon Sep 17 00:00:00 2001 From: Martijn Visser Date: Tue, 21 May 2024 15:19:25 +0200 Subject: [PATCH] Stronger 'Basin / profile' validation (#1486) Fixes #279. Follow up of #1469. Most rules were already in place, this just adds a bit more validation. I moved the validation out of the constructor to be more in line with the other validation taking place. --- core/src/parameter.jl | 38 ------------------------------------ core/src/read.jl | 5 +++++ core/src/validation.jl | 14 ++++++++----- core/test/validation_test.jl | 9 +++------ docs/core/usage.qmd | 4 ++-- 5 files changed, 19 insertions(+), 51 deletions(-) diff --git a/core/src/parameter.jl b/core/src/parameter.jl index a4ee97098..c94339779 100644 --- a/core/src/parameter.jl +++ b/core/src/parameter.jl @@ -186,44 +186,6 @@ struct Basin{T, C, V1, V2, V3} <: AbstractParameterNode demand::Vector{Float64} # Data source for parameter updates time::StructVector{BasinTimeV1, C, Int} - - function Basin( - node_id, - inflow_ids, - outflow_ids, - vertical_flux_from_input::V1, - vertical_flux::V2, - vertical_flux_prev::V3, - vertical_flux_integrated::V3, - vertical_flux_bmi::V3, - current_level::T, - current_area::T, - area, - level, - storage, - demand, - time::StructVector{BasinTimeV1, C, Int}, - ) where {T, C, V1, V2, V3} - is_valid = valid_profiles(node_id, level, area) - is_valid || error("Invalid Basin / profile table.") - return new{T, C, V1, V2, V3}( - node_id, - inflow_ids, - outflow_ids, - vertical_flux_from_input, - vertical_flux, - vertical_flux_prev, - vertical_flux_integrated, - vertical_flux_bmi, - current_level, - current_area, - area, - level, - storage, - demand, - time, - ) - end end """ diff --git a/core/src/read.jl b/core/src/read.jl index bad30b7df..9c2ac0517 100644 --- a/core/src/read.jl +++ b/core/src/read.jl @@ -571,6 +571,11 @@ function Basin(db::DB, config::Config, graph::MetaGraph, chunk_sizes::Vector{Int node_id = NodeID.(NodeType.Basin, node_id) + is_valid = valid_profiles(node_id, level, area) + if !is_valid + error("Invalid Basin / profile table.") + end + return Basin( Indices(node_id), [collect(inflow_ids(graph, id)) for id in node_id], diff --git a/core/src/validation.jl b/core/src/validation.jl index a682b4a89..40ecdee1b 100644 --- a/core/src/validation.jl +++ b/core/src/validation.jl @@ -186,16 +186,20 @@ end Check whether the profile data has no repeats in the levels and the areas start positive. """ function valid_profiles( - node_id::Indices{NodeID}, + node_id::Vector{NodeID}, level::Vector{Vector{Float64}}, area::Vector{Vector{Float64}}, )::Bool errors = false - for (id, levels, areas) in zip(node_id, level, area) + n = length(levels) + if n < 2 + errors = true + @error "$id profile must have at least two data points, got $n." + end if !allunique(levels) errors = true - @error "$id has repeated levels, this cannot be interpolated." + @error "$id profile has repeated levels, this cannot be interpolated." end if areas[1] <= 0 @@ -206,9 +210,9 @@ function valid_profiles( ) end - if areas[end] < areas[end - 1] + if any(diff(areas) .< 0.0) errors = true - @error "$id profile cannot have decreasing area at the top since extrapolating could lead to negative areas." + @error "$id profile cannot have decreasing areas." end end return !errors diff --git a/core/test/validation_test.jl b/core/test/validation_test.jl index 9184f09d7..2213e485b 100644 --- a/core/test/validation_test.jl +++ b/core/test/validation_test.jl @@ -1,10 +1,9 @@ @testitem "Basin profile validation" begin - using Dictionaries: Indices using Ribasim: NodeID, valid_profiles, qh_interpolation, ScalarInterpolation using Logging using StructArrays: StructVector - node_id = Indices([NodeID(:Basin, 1)]) + node_id = [NodeID(:Basin, 1)] level = [[0.0, 0.0, 1.0]] area = [[0.0, 100.0, 90]] @@ -12,18 +11,16 @@ with_logger(logger) do @test !valid_profiles(node_id, level, area) end - @test length(logger.logs) == 3 @test logger.logs[1].level == Error @test logger.logs[1].message == - "Basin #1 has repeated levels, this cannot be interpolated." + "Basin #1 profile has repeated levels, this cannot be interpolated." @test logger.logs[2].level == Error @test logger.logs[2].message == "Basin #1 profile cannot start with area <= 0 at the bottom for numerical reasons." @test logger.logs[2].kwargs[:area] == 0 @test logger.logs[3].level == Error - @test logger.logs[3].message == - "Basin #1 profile cannot have decreasing area at the top since extrapolating could lead to negative areas." + @test logger.logs[3].message == "Basin #1 profile cannot have decreasing areas." table = StructVector(; flow_rate = [0.0, 0.1], level = [1.0, 2.0], node_id = [5, 5]) itp = qh_interpolation(NodeID(:TabulatedRatingCurve, 5), table) diff --git a/docs/core/usage.qmd b/docs/core/usage.qmd index a76f603ab..e84002bdd 100644 --- a/docs/core/usage.qmd +++ b/docs/core/usage.qmd @@ -293,13 +293,13 @@ The profile table defines the physical dimensions of the storage reservoir of ea column | type | unit | restriction --------- | ------- | ------------ | ----------- node_id | Int32 | - | sorted -area | Float64 | $m^2$ | non-negative, per node_id: start positive and increasing +area | Float64 | $m^2$ | non-negative, per node_id: start positive and not decreasing level | Float64 | $m$ | per node_id: increasing The level is the level at the basin outlet. All levels are defined in meters above a datum that is the same for the entire model. An example of the first 5 rows of such a table is given below. The first 4 rows define the profile of ID `2`. The number of rows can vary -per ID. Using a very large number of rows may impact performance. +per ID, and must be at least 2. Using a very large number of rows may impact performance. node_id | area | level ------- |------- |-------