Flaw in Confinement tool logic identifies stretch as constricted when it is not

[Hornbydd]

The confinement tool is incorrectly identifying stretches of river as CONSTRICTED when it is not, see image below:

The flawed code is in the function transfer_line() in the ConfiningMargins.py module.

The problem is that the centroid from the confining margin is selecting the end of the split river centreline as shown by black lines because the centroid is inline with the end node of the confinement margin.

Subsequently the SelectLayerByLocation ends up selecting the u/s and d/s split lines attributing one of them with an incorrectly identified Confined value. This ends up cascading down the code and the stretch is given the wrong IsConfined and IsConstrict values.

[Hornbydd]

OK found another interesting scenario, here a tiny segment it is essentially collapsing to a null geometry and consequently tagging the u/s line as a right confinement when it is not.

I've had a good think about this looking at the code. The problem is that the original developer approached the issue of confinement with a "bulk process" approach, by which I mean they apply a selection on a dataset and call a tool that does the processing on the whole dataset. Nothing particularly wrong with this approach but it means that exceptions as I have found are almost impossible to resolve because the underlying logic is to process at the dataset level and not at the feature level.

As the points are coincident with the lines it is not possible to simply ask where along the line my particular point is as the tool is processing at the dataset level.

@joewheaton - To fix this bug would require a major rework of logic and process the geometries at the individual level rather than a bulk process approach. - Balls in your court on this one.

[Hornbydd]

Having studied the nightmare attribution of the intermediate datasets I might be able to apply a post fix solution. The split left/ right segments (which are later merged) have no attribution (ID values) that link them back to the splitting points as they are a result of a spatial selection and not an attribute selection. But I think I see a potential way, not sure about null geometries thought, will report any progress

[joewheaton]

Good catch @Hornbydd. This doesn't surprise me and comes out because a) the order in which development unfolded (confinement first, constriction and after thought, and b) the fact that constriction logic was never looked at that closely.

I had an idea that we would segment the network by all perpendicular intersections with confining margins on either side and then work through segment by segment to identify those three cases: confined on both sides, confined on one side, unconfined. After working out this geometry, you'd then want to be very clear about what to do next. If you want to report a 'constriction proportion (by length)' measure analogous to confinement, you'd then want to sum up the length of confined on both sides (constriction) for the original reach segments. However, the 'output' might just be these constricted reaches and there we might identify something else (e.g. their constriction ratio (i.e. width upstream to width @ constriction) or their expansion ratio (i.e. width @ constriction to width downstream). This is a full other feature and you and David and I should discuss it there.

[Hornbydd]

@joewheaton - OK some good news \bad news (with solution). I have written a function into the toolbox code that after it has assigned the left or right confinement flag (1) to the segment it post processes the data at a segment by segment level. This adds about 10 minutes on to the processing time for the Derwent. It essentially takes the segment, selects the near points, and uses those ID's to search the split points intermediate dataset. It add's orig_ids to a set and if the set ends up with a count of 1 then the line was correctly labelled otherwise if it ends up as 2 then there where multiple intersections which should not happen.

So the good news is that the code fixes the example above and few others. But a detailed scan of the output identifies a place where the logic breaks down as it is selecting 3 points. If you look at the image below you can see the red confinement margins are breaking up into small line segments, such that the original code is creating stacked points on the centre line caused by the near tool which is fundamental to the process.

Now I have a solution to this and you and Dave may not agree. In my eyes these tiny fragmented confinement margins (which are throughout the data) don't really represent anything other than spatial alignment of the input data. I could very easily put a new parameter on the tool with a minimum length filter. So after the lines are created by the intersection process the next step is to calculate their lengths and delete out anything less than say 10m in length. My gut feeling that this would solve immediately all but the most pathological cases!

I will upload my amended code now so you can look at the logic of my postFIXtoConfinement()

[Hornbydd]

After a discussion with David (5/9/18) I have implemented a filter Parameter as shown below.

This deletes any polylines created by the intersection process that are smaller than 5m. If user sets it to zero then none are deleted.

This has improved the output in some places but there are still issues with small lines collapsing to a null geometry creating stacked near points used for splitting the centre line and this interferes with the coding. These need to be resolved. As an example is shown below.

[joewheaton]

@Hornbydd it looks like adding that 'Filter by Length' parameter really helps. I completely agree with:

In my eyes these tiny fragmented confinement margins (which are throughout the data) don't really represent anything other than spatial alignment of the input data. I could very easily put a new parameter on the tool with a minimum length filter. So after the lines are created by the intersection process the next step is to calculate their lengths and delete out anything less than say 10m in length. My gut feeling that this would solve immediately all but the most pathological cases!

It seems to me you resolved this so I'm going to close this for now.