-
Notifications
You must be signed in to change notification settings - Fork 5
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
Offline coupling with Delwaq #1137
Conversation
Ah, you're starting to see why If I understand correctly, you want a routine that takes in the Ribasim model and spits out a new graph with edges only between basins and boundary nodes. You also want to know the flows over these edges. This might work (looking only at flow edges of course):
Using this representative edge system, I don't think any additional calculations or checks are needed. |
Ha, yes I understand your struggles. What you propose is indeed what I want, with the addition that the nodes need to be renumbered so they're consecutive. Same for the boundary nodes, but then negatively numbered. Is representative edge a name you came up with? You mean as an object to reason about (like done in the networkx package)? This approach requires directed edges, and I wonder whether it always holds. I can imagine that if the fractionalflows here would be nodes that allow bidirectional (negative) flow, a solution would be morw complex. Not sure if such configurations are possible with our current nodetypes. |
Yes, representative edge is a term I came up with, I haven't looked into the I'm not sure why the solution would be more complex with bidirectional edges. Do you want to convert to edges with only positive flow, so that you need edges in both directions? I wrote this function which I think does what you want: function create_graph_delwaq(graph::MetaGraph)::MetaGraph
graph_delwaq = MetaGraph(
DiGraph();
label_type = NodeID,
vertex_data_type = Int32,
# Representative edges
edge_data_type = Vector{Tuple{NodeID, NodeID}},
graph_data = nothing,
)
# Add all flow edges to the delwaq graph
for edge_metadata in values(graph.edge_data)
(; type, from_id, to_id) = edge_metadata
if type == EdgeType.flow
# These zeros are the delwaq IDs to be set later
graph_delwaq[from_id] = 0
graph_delwaq[to_id] = 0
edge = (from_id, to_id)
graph_delwaq[edge...] = [edge]
end
end
# Remove connector nodes from the delwaq graph
for node_id in collect(values(graph_delwaq.vertex_labels))
if node_id.type ∉ [
NodeType.Basin,
NodeType.Terminal,
NodeType.LevelBoundary,
NodeType.FlowBoundary,
NodeType.UserDemand,
]
inneighbor_id = only(inneighbor_labels(graph_delwaq, node_id))
outneighbor_ids = collect(outneighbor_labels(graph_delwaq, node_id))
# From MetagraphsNext
rem_vertex!(graph_delwaq, code_for(graph_delwaq, node_id))
for outneighbor_id in outneighbor_ids
edge = (inneighbor_id, outneighbor_id)
representative_edge = (node_id, outneighbor_id)
# If the new edge already exists, add a representative edge,
# otherwise create the new edge
if haskey(graph_delwaq, edge...)
push!(graph_delwaq[edge...], representative_edge)
else
graph_delwaq[edge...] = [representative_edge]
end
end
end
end
# Remap IDs
basin_id = 0
boundary_id = 0
for node_id in values(graph_delwaq.vertex_labels)
if node_id.type == NodeType.Basin
basin_id += 1
graph_delwaq[node_id] = basin_id
elseif node_id.type in [
NodeType.Terminal,
NodeType.UserDemand,
NodeType.LevelBoundary,
NodeType.FlowBoundary,
]
boundary_id -= 1
graph_delwaq[node_id] = boundary_id
else
error("Found unexpected node $node_id in delwaq graph.")
end
end
return graph_delwaq
end For the
and for a simple example where there are multiple paths between 2 basins, testmodel
Edit: Now you can also get the delwaq_id with
Don't mind the first integer in each tuple, that is the code created internally in MetaGraphsNext for each node. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Left a bunch of comments, mostly small. If they are addressed to the degree that they are applicable, it is good to merge from my side.
Co-authored-by: Martijn Visser <mgvisser@gmail.com>
Co-authored-by: Martijn Visser <mgvisser@gmail.com>
Quality work! |
Thanks, took me way longer that I would've liked, but very happy now that I've got it to work, including the CI. |
Forgot to add this in #1137, currently the website can't find this page.
Fixes #649
This PR adds a folder
coupling/delwaq
for offline coupling with Delwaq, with a more extensive description provided inREADME.md
in the same folder.A test is provided in
test.py
, that is also run on Teamcity on Windows, using the weekly DIMR releases. Thebasic
testmodel (the only with built-in substance concentration values) is used for this. In order to speed-up the CI one can now dopixi run ribasim-core-testmodels basic
to only run thebasic
model (but any or multiple model names can be used).