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

Stronger 'Basin / profile' validation #1486

Merged
merged 2 commits into from
May 21, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 0 additions & 38 deletions core/src/parameter.jl
Original file line number Diff line number Diff line change
Expand Up @@ -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

"""
Expand Down
5 changes: 5 additions & 0 deletions core/src/read.jl
Original file line number Diff line number Diff line change
Expand Up @@ -571,6 +571,11 @@

node_id = NodeID.(NodeType.Basin, node_id)

is_valid = valid_profiles(node_id, level, area)
if !is_valid
error("Invalid Basin / profile table.")

Check warning on line 576 in core/src/read.jl

View check run for this annotation

Codecov / codecov/patch

core/src/read.jl#L576

Added line #L576 was not covered by tests
end

return Basin(
Indices(node_id),
[collect(inflow_ids(graph, id)) for id in node_id],
Expand Down
14 changes: 9 additions & 5 deletions core/src/validation.jl
Original file line number Diff line number Diff line change
Expand Up @@ -186,16 +186,20 @@
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."

Check warning on line 198 in core/src/validation.jl

View check run for this annotation

Codecov / codecov/patch

core/src/validation.jl#L197-L198

Added lines #L197 - L198 were not covered by tests
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
Expand All @@ -206,9 +210,9 @@
)
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
Expand Down
9 changes: 3 additions & 6 deletions core/test/validation_test.jl
Original file line number Diff line number Diff line change
@@ -1,29 +1,26 @@
@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]]

logger = TestLogger(; min_level = Debug)
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)
Expand Down
4 changes: 2 additions & 2 deletions docs/core/usage.qmd
Original file line number Diff line number Diff line change
Expand Up @@ -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
------- |------- |-------
Expand Down