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

bf-traverse and dijkstra-traverse #10

Open
Engelberg opened this issue Dec 7, 2013 · 1 comment
Open

bf-traverse and dijkstra-traverse #10

Engelberg opened this issue Dec 7, 2013 · 1 comment
Assignees

Comments

@Engelberg
Copy link
Contributor

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.
@aysylu
Copy link
Owner

aysylu commented Dec 14, 2013

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants