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

JGraphT #323

Open
pnrobinson opened this issue Jan 29, 2021 · 10 comments · Fixed by #448
Open

JGraphT #323

pnrobinson opened this issue Jan 29, 2021 · 10 comments · Fixed by #448
Labels
enhancement New feature or request
Milestone

Comments

@pnrobinson
Copy link
Member

phenol is at version 1.2.0. There are API changes for version 1.5.0 (the current one). Investigate how to update.

@julesjacobsen
Copy link
Contributor

Migrate to Guava? I think it has a nicer API to JGraphT, but that might no longer be the case with 1.5.0. Sorry, that wasn't helpful!

@pnrobinson
Copy link
Member Author

Didn't know that guava has a graph library. If so we could switch to save a dependency
We do not do anything really sophisticated with graphs, and it might be simpler just to implement this ourselves.
We need to do traversals mainly.

@julesjacobsen
Copy link
Contributor

@julesjacobsen
Copy link
Contributor

It has a ValueGraph implementation

MutableValueGraph<Integer, Double> weightedGraph = ValueGraphBuilder.directed().build();
weightedGraph.addNode(1);
weightedGraph.putEdgeValue(2, 3, 1.5);  // also adds nodes 2 and 3 if not already present
weightedGraph.putEdgeValue(3, 5, 1.5);  // edge values (like Map values) need not be unique
...
weightedGraph.putEdgeValue(2, 3, 2.0);  // updates the value for (2,3) to 2.0

and I think Phenol has its own traversal code, right?

@julesjacobsen
Copy link
Contributor

However JGraphT has sparse representations for immutable graphs, (https://github.com/jgrapht/jgrapht/tree/master/jgrapht-opt/src/main/java/org/jgrapht/opt/graph/sparse) so it might be worth looking into this although these don't appear to support generic value graphs. I had a go at creating a CSR graph based on the Guava API some time back but never quite got it finished :( I could try and dig-up the code if you like.

@pnrobinson
Copy link
Member Author

CSR would be great!
I implemented one in Python that worked.
We could probably do this in pure Java, to reduce dependencies; We should check where and how TGraph is being used in phenol and figure out how much work this would be and whether we need a library.

@ielis ielis added this to the v3.0.0 milestone Nov 3, 2023
@ielis ielis added the enhancement New feature or request label Nov 3, 2023
@ielis ielis linked a pull request Nov 4, 2023 that will close this issue
@ielis
Copy link
Member

ielis commented Nov 4, 2023

Thanks to the recent development, we have a pure Java CSR graph implementation and unless we have really good reasons, I will remove JGraphT without replacement in v3.0.0.

@ielis
Copy link
Member

ielis commented Nov 20, 2023

#450

@pnrobinson
Copy link
Member Author

@ielis I think we can probably go ahead and just remove the old APIs, let's simplify things and move towards a first documented release next year! Are we ready to remove the jgraphT import?

@ielis
Copy link
Member

ielis commented Jul 2, 2024

There is one site left in Phenol which uses the JGraphT graph: the ShortestPathTable.

Otherwise, getGraph() is not used anywhere else, hence ready to be removed. The removal will be part of the next major release - v3.

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

Successfully merging a pull request may close this issue.

3 participants