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

Add Junction node #1609

Open
SouthEndMusic opened this issue Jul 4, 2024 · 20 comments
Open

Add Junction node #1609

SouthEndMusic opened this issue Jul 4, 2024 · 20 comments
Labels
DSD Preparations for Delft Software Days needs-refinement Issues that are too large and need refinement

Comments

@SouthEndMusic
Copy link
Collaborator

It came out of discussions with @Fati-Mon that it would be nice to have a confluence node, which I think exists in Ribasim 7-8. It comes down to a node where flows from multiple edges can come in, which is then summed on one outgoing edge. Currently we only support something like this by bringing these multiple flows together into a basin, but the downside of this is that you then need profile information for this basin.

@SouthEndMusic SouthEndMusic added enhancement needs-refinement Issues that are too large and need refinement labels Jul 4, 2024
@github-project-automation github-project-automation bot moved this to To do in Ribasim Jul 4, 2024
@visr visr removed the enhancement label Nov 26, 2024
@SnippenE SnippenE added the DSD Preparations for Delft Software Days label Dec 9, 2024
@evetion
Copy link
Member

evetion commented Mar 17, 2025

We decided to add a Confluence node. It can come after any directional connector node, and before a Basin. They can be chained. @SouthEndMusic will now check how this design effects our core implementation(s).

Relates to making ManningResistance directional (#2142).

There was some discussion on how this relates to Routing (#2129). We decided to keep these concepts separate, and a Delay node would be a connector node like ManningResistance.

@visr
Copy link
Member

visr commented Mar 17, 2025

In terms of https://ribasim.org/reference/validation.html

  • flow_in_min: 1
  • flow_in_max: ∞
  • flow_out_min: 1
  • flow_out_max: 1
  • control_in_min: 0
  • control_in_max: 0
  • control_out_min: 0
  • control_out_max: 0

Allowed upstream neighbors:

  • confluence
  • linear_resistance (one-way)
  • manning_resistance (one-way)
  • tabulated_rating_curve
  • flow_boundary
  • pump
  • outlet

Allowed downstream neighbors:

  • confluence
  • basin
  • terminal

@gijsber gijsber moved this from To do to Sprint backlog in Ribasim Mar 20, 2025
@visr
Copy link
Member

visr commented Mar 21, 2025

Some more thoughts.

We often make links follow the center of water bodies, since they connect Nodes, not the edge of a Basin / area. That means we get links that lay on top of one another in parts of waterbodies, such as the 3 on the left top below. They each represent one part of the flow.

One way to relieve this is to offer a topological view in our viewers with straight links, that do not follow waterbodies.

Confluences can also be used to avoid this visualization and interpretation issue, see the example below:

Image

The Confluence probably becomes easier to implement if we enforce one way flow over it. But it would also be less useful, as it cannot generally be used in such a way. So if feasible I'd like to lift that requirement, and keep that in #2142.

The way to interpret the connections in the image, where X is a Basin and A/B/C connector nodes, is that these are the actual connections:

  1. A->X
  2. B->X
  3. C->X

So there are no direct connections between A, B and C, all flow goes via X. In that sense these Confluence nodes can possibly not exist in the core and be computed afterwards for the results if that makes things simpler. Assuming all links are directed towards the Basin then the right Confluence and brown mid segment have Qax+Qbx, and the left Confluence and blue left segment have Qax+Qbx+Qcx.

@visr
Copy link
Member

visr commented Mar 25, 2025

Just looking at the HWS network with @DanielTollenaar I realized that the Confluence node would also be nice to have on the bifurcation of the links here:
Image

Putting Confluences on bifurcations is another confusing situation just like using Outlets as inlets. @DanielTollenaar suggested the node name Connection instead of Confluence so we cover both. The connection node also exists in SOBEK.

@Fati-Mon
Copy link
Collaborator

Correct me if I am wrong, but the arrow suggest the flow goes from basin to the connector nodes? I thought the purpose of the confluence node was to have a single flow emerging from it. In the case of bifurcation, wouldn't that idea be disrupted? Or could the 'connection' node support multiple outflows instead? Also, the term 'Connection' node might be a bit unclear, especially since we already use 'Connector nodes.' I've noticed some confusion when people refer to 'Flow Boundary' but mean 'Flow Demand.' We really need to think carefully about the naming conventions to make them more intuitive so that modelers can easily understand what each term represents.

@evetion
Copy link
Member

evetion commented Mar 25, 2025

Yeah we discussed that this morning, let's go with connection. This also strengthens the case for putting it outside of the solver, as bifurcation is yet another beast to implement.

@evetion
Copy link
Member

evetion commented Mar 25, 2025

@Fati-Mon, good points, would you be ok with Confluence and Bifurcation?

After giving this some thought, a Connection is a real pain to implement. We can simplify topology in case of a N:1 connection (Confluence), or a 1:N connection (Bifurcation). However, if we make a Connection (or allow a Confluence Bifurcation connection) this becomes N:N and we cannot determine how to divide the flow.

@visr
Copy link
Member

visr commented Mar 25, 2025

We can simplify topology in case of a N:1 connection (Confluence), or a 1:N connection (Bifurcation).

I think those cases would be enough? I can imagine N:M becomes complicated but I don't see a need for it.

Having a Connection node can be confusing with connector node. Though connector node is not a real this just a name that we started using, and we could change that in the docs to avoid confusion with Connection nodes.

@evetion
Copy link
Member

evetion commented Mar 25, 2025

I think those cases would be enough? I can imagine N:M becomes complicated but I don't see a need for it.

Yes, but as separate nodetypes right?

@visr
Copy link
Member

visr commented Mar 25, 2025

Why? We don't care about the link direction here, and the 1 in 1:N and N:1 is always the Basin, so it is the same thing.

@evetion
Copy link
Member

evetion commented Mar 25, 2025

No, you can place multiple ones after another (the N being another Connection, per your own proposal above), making it N:N.

@gijsber
Copy link
Contributor

gijsber commented Mar 25, 2025

Alternative node name for Connection: Junction

@visr
Copy link
Member

visr commented Mar 25, 2025

Junction is probably more intuitive than Connection.

making it N:N

Do you have examples? It is hard to talk about this but I meant 1:N or N:1 with 1 being a Basin in a view where you collapse the Junction nodes, so from the bottom image to the top image in #1609 (comment).

@visr
Copy link
Member

visr commented Mar 25, 2025

Also the argument that @DanielTollenaar brought up to combine Confluence and Bifurcation is that it is not always clear which one it is, imagine a T junction in a polder, where flows can reverse.

@evetion
Copy link
Member

evetion commented Mar 25, 2025

Yeah let's draw it in the office next Thursday. But a quick example nonetheless:

Loading
flowchart LR
    A((B)) --> C{TR}
    B((B)) --> D{TR}
    C{TR} --> E[/J\]
    D{TR} --> E[/J\]
    E[/J\] --> F[/J\]
    F --> G((B))
    F --> H((B))

edit: Updated it

@visr
Copy link
Member

visr commented Mar 25, 2025

I think that's valid if exactly 1 of those 4 blue circles is a Basin, doesn't matter which one.

@gijsber
Copy link
Contributor

gijsber commented Mar 25, 2025

Good to see that the name Junction is quickly getting traction. Apparently a good name :-)

@SouthEndMusic
Copy link
Collaborator Author

I think it's best to think in terms of an iterative procedure where you remove confluence nodes in the core one by one from the graph (let's call it reducing the graph):

Each time you remove a Junction node, you connect all inflow neighbours to all outflow neighbours. If that means connecting 2 non Junction nodes, validate that connection.

I think it's nice to build up a sparse matrix during this procedure, which specifies the relationship between the remaining links and the original links. This can then be used to compute the output flows, where some of them are cumulative.

To make this work with listening to a link that contains cumulative flow by ContinuousControl, this flow has to be expanded into a sum of flows in the reduced graph (as given by the sparse matrix mentioned above).

@visr visr changed the title Add Confluence node Add Junction node Mar 25, 2025
@evetion
Copy link
Member

evetion commented Mar 26, 2025

I think it's best to think in terms of an iterative procedure where you remove confluence nodes in the core one by one from the graph (let's call it reducing the graph):

Each time you remove a Junction node, you connect all inflow neighbours to all outflow neighbours. If that means connecting 2 non Junction nodes, validate that connection.

I think it's nice to build up a sparse matrix during this procedure, which specifies the relationship between the remaining links and the original links. This can then be used to compute the output flows, where some of them are cumulative.

That's exactly what I was implementing, although I still have to rename Confluence 😉

To make this work with listening to a link that contains cumulative flow by ContinuousControl, this flow has to be expanded into a sum of flows in the reduced graph (as given by the sparse matrix mentioned above).

Can we make an issue for expanding ContinuousControl? I rather expand it first on its own without Junction to fully test and understand it.

@SouthEndMusic
Copy link
Collaborator Author

SouthEndMusic commented Mar 26, 2025

Can we make an issue for expanding ContinuousControl? I rather expand it first on its own without Junction to fully test and understand it.

I don't understand fully what you mean here, but it's fine by me to do the ContinuousControl part separately. For that, the only thing you have to do is to make sure during initialization that what is a single variable in the input (https://ribasim.org/reference/node/continuous-control.html#variable) becomes n separate variables multiplied by the same weight. In fact, the exact same thing has to be done for DiscreteControl.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
DSD Preparations for Delft Software Days needs-refinement Issues that are too large and need refinement
Projects
Status: Sprint backlog
Development

No branches or pull requests

6 participants