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

Bug: ArrayBasedDirectedGraph in OutDir(InDir) should not create nodes that have no out (in) edges #169

Open
pankajgupta opened this issue Feb 27, 2015 · 5 comments
Assignees
Labels

Comments

@pankajgupta
Copy link
Contributor

No description provided.

@pankajgupta pankajgupta self-assigned this Feb 27, 2015
@pankajgupta
Copy link
Contributor Author

It is harsh to call this a bug, just a missed optimization.

@pankajgupta
Copy link
Contributor Author

Started fixing this and realized that this would need changes to Traverser as well. With this change, for a graph with a single edge 1->2, only node 1 will exist in the graph. This breaks traverser as it currently needs node 2 to be present as well. The best choice would be to change Traverser to be an iterator of node-ids, not nodes. Any thoughts @szymonm ?

@szymonm
Copy link
Contributor

szymonm commented Feb 27, 2015

IIUC, after fix getNodeById will return Node but the node is going to be instantiated on the fly with no edges, right?
Thit is something different than returning None, when there is no such node in the graph, right?
So shouldn't Traverser return the Node (with empty edges list) that was created by getNodeById?

I think returning a Node is more general and easier to use than just id, i.e., we will have to pass graph whenever we want to use only nodes generated by a Traverser. Consider for example writing methods that use DFS tree generated by a traverser. If Traverser <: Iterator[Int] we have to pass additionally graph to every such method.
Also, in traverser we have to call getNodeById for every returned node/id. This operation may be expensive if nodes are stored in other data structure than array. In particular,when using dristributed graph: we don't want to call getNodeById (again), because that means fetching node again over the network.

@pankajgupta
Copy link
Contributor Author

I wasn't thinking that getNodeById will construct it on the fly but what you say makes sense. Thanks.

@szymonm
Copy link
Contributor

szymonm commented Jan 26, 2016

Does this means that it's not a bug?

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

No branches or pull requests

2 participants