Deltares / Ribasim

Water resources modeling
https://deltares.github.io/Ribasim/
MIT License
36 stars 5 forks source link

Remove FractionalFlow node #1582

Open visr opened 3 days ago

visr commented 3 days ago

The FractionalFlow node is simple since the outflow is a fraction of the inflow. But behind this simplicity hides a lot, making it the least popular node among developers.

At the same time we already have plenty of options to bifurcate water, by adding multiple Pumps, TabulatedRatingCurves etc. to a single Basin. These cover the use cases of FractionalFlow.

SouthEndMusic commented 3 days ago

One thing to consider is that currently allocation adapts to changes in fractional flow fractions by discrete control. We should think about whether we still want to support something like that.

gijsber commented 3 days ago

We also need to add examples how you would mimic fractional flows by TabulatedRatingCurves, Pumps or outlets. Titles could as descriptive as: Implementing FractionFlows using TabulatedRatingCurves

Huite commented 2 days ago

But behind this simplicity hides a lot, making it the least popular node among developers.

I understand why it's the least popular among developers, but how is its popularity amongst the users? It's true that you can replace fractional flow by a set of coordinated TabulatedRatingCurves, but it feels like you're just distributing the labor. I would guess that splitting the water is a relatively common way to schematize flow networks. By removing the node, aren't you just shifting the problem to the user? Won't they run into the same problems you describe here?

If I take a look at our examples:

image

Basin 3 discharges via TBC 4, which could represent a weir. After the weir, the water goes through a bifuraction (trifurcation). The TBC can contain a somewhat detailed rating curve (say it has 20 Q-h vertices); for the trifurcation, it needs to be triplicated, scaling the proscribed discharges with the flow fraction for each. Then, to implement control measures, each needs to be scaled again in coordination with each other such that the total Q still represents the weir at 4.

The current implementation has:

A fully TBC'ed implementation has:

With four control conditions, you're looking at 32 versus 240 input rows, and the behavior of the weir at 4 is scattered over 3 tables.

Thinking out loud, to facilitate this:

Is some kind of a composite node an option? The current FractionalNode can only be connected to something that spits out a Q. So it doesn't work for a LinearResistance or ManningResistance node, but it will work for Pump, TabulatedRatingCurve, and the Outlet. So the required nodes would be BifurcatingPump, BifurcatingTabulatedRatingCurve, and BifurcatingOutlet. The pump and outlet are mostly pointless, since you're just specifying a single Q anyway. Although, a nice feature with the current implementation is that you need only a single control connection to set the fractions. With N pumps, you need N control connections too...

So you could manage somewhat with just a BifurcatingTabulatedRatingCurve, which has a fraction entry or something? It still doesn't feel like the right thing to me, though.

visr commented 2 days ago

I forgot to add that I discussed this with key users yesterday. Indeed some of the incentive for this comes from developer frustration, but I think what this really indicates is that the concept is not right, it does not go well with the rest of the physical layer, largely since we care about water levels.

So it doesn't work for a LinearResistance or ManningResistance node, but it will work for Pump, TabulatedRatingCurve, and the Outlet.

It doesn't work for an Outlet, because that doesn't flow uphill. If one of the downstream Basins has higher water levels, the fractions cannot be maintained. This came up recently in https://github.com/Deltares/Ribasim/issues/1567#issuecomment-2180585207. For Pump this currently works, but wouldn't work anymore if we'd add some max head difference. For TabulatedRatingCurve this currently works, but we intend to set the Q to 0 if the downstream Basin has higher water levels. So if we apply those, it wouldn't work with any nodes except FlowBoundary.

A fractional division is indeed something water managers talk about. And for an even more conceptual model without water levels it makes a lot of sense. But for us I feel like a fraction is more of a result on a bifurcation that tends to hold in most cases. A Q(Q) function, that sends a flow down based on another flow, can achieve the same results. This can be done with a controlled Outlet. In Ribasim 7 the Bifurcation node works like that, but is hardcoded to listen to the flow directly upstream. In NL many of these are Q(Qlobith).

Huite commented 2 days ago

It doesn't work for an Outlet, because that doesn't flow uphill.

Fair point.

I concur regarding the key issue: the fraction belongs conceptually in the control layer, not in the physical layer.

The current problem is that DiscreteControl doesn't allow for very smooth Q = f(Q) functions, right? I think the key part in making this easier this might be a (Piecewise)LinearControl (or something...)?

Which would be useful to have for Q = f(H) too (and which has been requested before).

In that case, you indeed set a number of Outlets, get them to listen to some upstream flow. For a trifurcation with 40-40-20 split, you could simply define (Qupstream, Qoutflow) for the three outlets:

1: (0.0, 0.0), (1000.0, 400.0)

  1. (0.0, 0.0), (1000.0, 400.0)
  2. (0.0, 0.0), (1000.0, 200.0)

I.e. no outflow when Q upstream is 0, and otherwise fraction * Qupstream. If fractions need to adjust for higher flow conditions to get a 45-45-10 split, you can add another vertex:

1: (0.0, 0.0), (1000.0, 400.0), (2000.0, 900.0)

  1. (0.0, 0.0), (1000.0, 400.0), (2000.0, 900.0)
  2. (0.0, 0.0), (1000.0, 200.0), (2000.0, 200.0)

This does mean that effective fractions are interpolated and thus a continuous function (unlike what you'd currently get with DiscreteControl and hard fractions); i.e. with upstream flow at 1500, you'd get outflows of: 650, 650, 200, or a 43-43-13 split. If people are deadset on fixed fractions, they essentially have to splice in another Qupstream value:

1: (0.0, 0.0), (1000.0, 400.0), (1100, 495), (2000.0, 900.0)

  1. (0.0, 0.0), (1000.0, 400.0), (1100, 495), (2000.0, 900.0)
  2. (0.0, 0.0), (1000.0, 200.0), (110, 110), (2000.0, 200.0)

This seems manageable?

evetion commented 1 day ago

I concur regarding the key issue: the fraction belongs conceptually in the control layer, not in the physical layer.

It might even be in the allocation layer ;)

My two cents, indeed the current code implementation of fractional flow feels bad (I've seen an if fractionalflow somewhere in the hot loop code), and Bart has said it's his nemesis (as it breaks the assumption that everything in the graph is just a single node away). So if we can make it more elegant or remove it, great! However, it should be first and foremost a user request.

I forgot to add that I discussed this with key users yesterday. Indeed some of the incentive for this comes from developer frustration, but I think what this really indicates is that the concept is not right, it does not go well with the rest of the physical layer, largely since we care about water levels.

Can you detail what their frustration is about? Is it hard to setup? Or does it work differently from what they expect?

So if we apply those, it wouldn't work with any nodes except FlowBoundary.

Would that still be useful to the end user? If so, let's do it! It's the simplest and most correct form, and it requires just updating validation rules.

In my opinion (and from reading the documentation!?) another good improvement is to have a single FractionalFlow, instead of multiple ones for the same xfurcation.

But before we go a complex route (I do like @huite combined node types, but am firmly against replacing nodes automatically), let's go back to the drawing board and write down the use case(s).