Skip to content

Use igraph as underlying graph library

Vicentini Filippo requested to merge igraph into master

Created by: femtobit

This is a draft PR which partially addresses #727 (closed). (Note that this is currently a PR on a separate working branch in this repo which is based on the current state of #724. This is just so the changes show up correctly here so we can already discuss them. This PR should re-targeted and merged only after the lattice PR went through.)

Main changes:

  • Rename NetworkX to Graph and merge the old Graph function with its constructor.
  • Use igraph.Graph internally in Graph, which speeds up some computations. Incidentally, this also means Graph.automorphisms now works with colored edges which was unsupported by the networkx implementation previously.
  • Makes igraph a base dependency of NetKet, obviously.

While the internal graph is not exposed, Graph now does provide two method pairs from_igraph/to_igraph and from_networkx/to_networkx (they are easy to implement, regardless of our underlying implementation). Once networkx is not a hard dependency anymore, we can still leave to_networkx which will only work if the user optionally installs networkx. which seems pretty natural to me (we also have this with matplotlib in Lattice.draw).

What is not (yet) done here:

  • Remove the dependency on networkx outside of the tests.

There is one other change to discuss:

  • Vertices are now always labeled by range(0, n_nodes) in Graph, without support for custom indices. I think this is a good change, because this is what we supported in v2 (as far as I remember) and random parts of our code still assume this and will break if the nodes are represented by something else. Note that if users want to associate names or any other objects with vertices, they can just put those in list of length n_nodes easily anyways.

Merge request reports