BUG: Avulsion setup and pathfinding

[amoodie]

Found a bug that we need to decide how to deal with.

Consider, checkAuvlsions identifies that both cells 5661 and 5762 should have an avulsion, and that both avulsions should go to 5763. So, processing the avulsions begins with enactAvulsions and the link from 5661 to 5763 is made (image left). Then, the propagateAvulsion routes the channel to 5762 (image right) and the routing stops, because it has reached a channel cell. Then, we return to enactAvulsions to execute the next avulsion, which links 5762 to 5763 and forms a loop.

Now, the loop should be able to be abandoned in the next step (there's another bug there that led me to find this one), but we should avoid creating the loop in the first place.

One solution could be to change the avulsion enacting from check for all avulsions, then enact all avulsions, to something like check for an avulsion, enact that avulsion, check for another avulsion. The current implementation is also not true parallel avulsions, so I don't think this a big conceptual change, but would require some code change and refactoring.

Another solution could be to check somewhere around here just before making the avulsion connection that we still want to make the connection and avulsion (e.g., that it would not form a loop). This means more checking of the flowsTo and flowsFrom arrays, so it means more code complexity, but it may be more straightforward to implement.

Thoughts?

[amoodie]

A more general solution that may help streamline both this issue and #27 could be to stop deciding where to route the avulsion during avulsionCheck and just flag that an avulsion should happen out of that cell. Then in enactAvulsions and propogateAvulsions we can decide where (and whether at all) to route the avulsion.

This is attractive to me in terms of streamlining the codebase (remove one set of neighbor slope calculating code). But this change alone does not answer the questions in this and #27.

[alimaye]

Agree with proposed change to use avulsionCheck only to flag for avulsions (that may or may not be enacted in subsequent functions).

[amoodie]

I can't reproduce the example from above to see if this has actually been fixed, but I think the changes in #28 took care of it.

There, we set it so that the step of an avulsion (the first or any other step) cannot be into any cell that is currently directing flow into that cell.

This would mean the second avulsion in the above example would be routed elsewhere and not form a loop. I'm going to close this now, and we can open another issue if we find the issue persists.