Skip to content

Commit

Permalink
Re-enable edge validation (#888)
Browse files Browse the repository at this point in the history
This validation was inadvertently not called anymore. This now adds a
test to make sure that doesn't happen again.

@SouthEndMusic tests fail since an invalid connection slipped in since,
do you want to take over this PR to fix that?

```
┌ Error: Cannot connect a basin to a basin.
│   edge_id = 0
│   id_src = #2
│   id_dst = #4
└ @ Ribasim D:\Ribasim\core\src\validation.jl:450
Error in testset "Allocation objective types" on worker 24444:
Error During Test at D:\Ribasim\core\test\allocation_test.jl:27
```

---------

Co-authored-by: Bart de Koning <[email protected]>
  • Loading branch information
visr and SouthEndMusic authored Dec 15, 2023
1 parent aa243c8 commit 641758f
Show file tree
Hide file tree
Showing 5 changed files with 21 additions and 7 deletions.
4 changes: 4 additions & 0 deletions core/src/create.jl
Original file line number Diff line number Diff line change
Expand Up @@ -794,6 +794,10 @@ function Parameters(db::DB, config::Config)::Parameters
graph = create_graph(db, config, chunk_sizes)
allocation_models = Vector{AllocationModel}()

if !valid_edges(graph)
error("Invalid edge(s) found.")
end

linear_resistance = LinearResistance(db, config)
manning_resistance = ManningResistance(db, config)
tabulated_rating_curve = TabulatedRatingCurve(db, config)
Expand Down
1 change: 1 addition & 0 deletions core/src/utils.jl
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,7 @@ function create_graph(db::DB, config::Config, chunk_sizes::Vector{Int})::MetaGra
flow_vertical,
)
graph = @set graph.graph_data = graph_data

return graph
end

Expand Down
2 changes: 1 addition & 1 deletion core/src/validation.jl
Original file line number Diff line number Diff line change
Expand Up @@ -447,7 +447,7 @@ function valid_edges(graph::MetaGraph)::Bool
if !(type_dst in neighbortypes(type_src))
errors = true
edge_id = graph[id_src, id_dst].id
@error "Cannot connect a $type_src to a $type_dst (edge #$edge_id from node $id_src to $id_dst)."
@error "Cannot connect a $type_src to a $type_dst." edge_id id_src id_dst
end
end
return !errors
Expand Down
12 changes: 9 additions & 3 deletions core/test/validation_test.jl
Original file line number Diff line number Diff line change
Expand Up @@ -219,8 +219,8 @@ end
config = Ribasim.Config(toml_path)
db_path = Ribasim.input_path(config, config.database)
db = SQLite.DB(db_path)
p = Ribasim.Parameters(db, config)
(; graph, fractional_flow) = p
graph = Ribasim.create_graph(db, config, [1, 1])
fractional_flow = Ribasim.FractionalFlow(db, config)

logger = TestLogger()
with_logger(logger) do
Expand All @@ -229,9 +229,10 @@ end
fractional_flow.node_id,
fractional_flow.control_mapping,
)
@test !Ribasim.valid_edges(graph)
end

@test length(logger.logs) == 3
@test length(logger.logs) == 4
@test logger.logs[1].level == Error
@test logger.logs[1].message ==
"Node #7 combines fractional flow outneighbors with other outneigbor types."
Expand All @@ -247,6 +248,11 @@ end
@test logger.logs[3].kwargs[:node_id] == NodeID(7)
@test logger.logs[3].kwargs[:fraction_sum] 0.4
@test logger.logs[3].kwargs[:control_state] == ""
@test logger.logs[4].level == Error
@test logger.logs[4].message == "Cannot connect a basin to a fractional_flow."
@test logger.logs[4].kwargs[:edge_id] == 7
@test logger.logs[4].kwargs[:id_src] == NodeID(2)
@test logger.logs[4].kwargs[:id_dst] == NodeID(8)
end

@testitem "DiscreteControl logic validation" begin
Expand Down
9 changes: 6 additions & 3 deletions python/ribasim_testmodels/ribasim_testmodels/invalid.py
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,7 @@ def invalid_fractional_flow_model():
(0.0, -2.0), # 5: Terminal
(0.0, 2.0), # 6: Terminal
(0.0, 0.0), # 7: TabulatedRatingCurve
(-1.0, -1.0), # 8: FractionalFlow
]
)
node_xy = gpd.points_from_xy(x=xy[:, 0], y=xy[:, 1])
Expand All @@ -120,6 +121,7 @@ def invalid_fractional_flow_model():
"Terminal",
"Terminal",
"TabulatedRatingCurve",
"FractionalFlow",
]

# Make sure the feature id starts at 1: explicitly give an index.
Expand All @@ -134,8 +136,8 @@ def invalid_fractional_flow_model():

# Setup the edges:
# Invalid: Node #7 combines fractional flow outneighbors with other outneigbor types.
from_id = np.array([1, 7, 7, 3, 7, 4], dtype=np.int64)
to_id = np.array([7, 2, 3, 5, 4, 6], dtype=np.int64)
from_id = np.array([1, 7, 7, 3, 7, 4, 2], dtype=np.int64)
to_id = np.array([7, 2, 3, 5, 4, 6, 8], dtype=np.int64)
lines = node.geometry_from_connectivity(from_id, to_id)
edge = ribasim.Edge(
df=gpd.GeoDataFrame(
Expand Down Expand Up @@ -177,7 +179,8 @@ def invalid_fractional_flow_model():
# Setup the fractional flow:
fractional_flow = ribasim.FractionalFlow(
# Invalid: fractions must be non-negative and add up to approximately 1
static=pd.DataFrame(data={"node_id": [3, 4], "fraction": [-0.1, 0.5]})
# Invalid: #8 comes from a Basin
static=pd.DataFrame(data={"node_id": [3, 4, 8], "fraction": [-0.1, 0.5, 1.0]})
)

# Setup the tabulated rating curve:
Expand Down

0 comments on commit 641758f

Please sign in to comment.