loom icon indicating copy to clipboard operation
loom copied to clipboard

bf-traverse and dijkstra-traverse

Open Engelberg opened this issue 12 years ago • 1 comments

I just looked through the loom source code tonight, enjoyed reading it. A few thoughts:

  1. I'm not crazy about how bf-traverse and dijkstra-traverse return very different types of things. The former just returns a seq of nodes, and the latter returns a seq of [node state]. If you pass in an optional function, the function takes different kinds of information for the two types of traversals. Would like to see these be more similar: same interface for both but one uses depth where the other uses distance.
  2. A common need is to do a traversal out from some source, returning the final predecessor map and the final depth/distance map. Would like to see this exposed in alg.clj as a convenient function, for example, something like: (shortest-path-preds g start) -> [preds-map dist-map] where preds-map is a map of the form {node predecessor} and dist-map is a map of the form {node depth/distance-from-start}.
  3. Your "poor man's priority queue" in alg-generic/dijkstra-traverse is flawed. It is theoretically possible that two different, uncomparable nodes could have equal distance and equal hash. It's an unlikely scenario, but would cause a crash. I suggest using priority-map (similar speed because under the hood it also uses sorted-set, but handles these edge cases correctly) or if you really want maximum speed, a mutable java priority queue.

Engelberg avatar Dec 07 '13 07:12 Engelberg

Those are all very good points, thank you! I don't think I'll have time to get to this until the end of the year, though. However, pull requests with these changes are welcome.

aysylu avatar Dec 14 '13 02:12 aysylu