qiskit-community / qiskit-nature

Qiskit Nature is an open-source, quantum computing, framework for solving quantum mechanical natural science problems.
https://qiskit-community.github.io/qiskit-nature/
Apache License 2.0
306 stars 207 forks source link

Lattice does not support all rustworkx graph representations. #1347

Open MarcoBarroca opened 9 months ago

MarcoBarroca commented 9 months ago

Environment

What is happening?

When creating a Lattice from a rustworkx graph Lattice() only supports graphs of the form

graph = rx.PyGraph(multigraph=False)
graph.extend_from weighted_edge_list(
    [(0, 1, 1), (0, 2, 2),
     (1, 3, 2), (3, 0, 3)])

rustworkx support two types of weighted graph representations. The above or

graph = rx.PyGraph(multigraph=False)
graph.extend_from weighted_edge_list(
    [(0, 1, {'weight': 1}), (0, 2, {'weight': 2}),
     (1, 3, {'weight': 2}), (3, 0, {'weight': 3})])

Using the second one results in

ValueError: Unsupported weight {'weight': 1} on edge with index 0.

How can we reproduce the issue?

This should reproduce it with the necessary imports:

graph = rx.PyGraph(multigraph=False)
graph.extend_from_weighted_edge_list(
    [(0, 1, {'weight': 1}), (0, 2, {'weight': 2}),
     (1, 3, {'weight': 2}), (3, 0, {'weight': 3})])

lattice = Lattice(graph)

What should happen?

A lattice should be constructed given a rustworkx graph in any of the two forms.

Any suggestions?

Either modify lattice.py to take both into account or modify the graph object to be of the correct format.

mrossinek commented 7 months ago

rustworkx graph cans actually store arbitrary data as payloads of their nodes and edges. You cannot possibly cover all possible such payloads and expect the code to extract a weight from it.

That said, the payload which is supported (i.e. just a plain weight) should probably be documented properly. Right now, the only mention of this fact is here: https://github.com/qiskit-community/qiskit-nature/blob/cbba76c516cbf372903a336b17fdeb2c31cd6d57/qiskit_nature/second_q/hamiltonians/lattices/lattice.py#L128

Therefore, this is either a documentation issue or a feature request to support more types although I would not be sure how this would work in a general case because the Hamiltonian generators which use the lattice would need to be adapted to interpret non-numeric payloads, too.

MarcoBarroca commented 7 months ago

This is relevant because networkx graphs only store weights as values in the dictionary and if Lattice() is given a NetworkX graphs it does the following:


if not isinstance(graph, PyGraph):
            _optionals.HAS_NETWORKX.require_now("Lattice construction from networkx.Graph")
            graph = networkx_converter(graph)

The network_converter() will keep the format networkx uses into the rustworkx graph.

I stumbled into this issue trying to take a weighted graph that was built elsewhere with NetworkX into a Lattice(). I had to manually convert it to rustworkx to make sure it wasn’t using a dictionary for the weights before building the lattice.

MarcoBarroca commented 7 months ago

Here is an example.

G = nx.Graph()
edges=[[1,2],[0,3],[2,3],[0,1]]
for edge in edges:
    edge_1 = edge[0]
    edge_2 = edge[1]
    weight = 1.0
    G.add_edge(edge_1, edge_2,weight=weight)

general_lattice = Lattice(G)

Gives ValueError: Unsupported weight {'weight': 1.0} on edge with index 0.

To fix it we have to manually convert it to an adequately formatted rustworkx object. This works:

G = nx.Graph()
edges=[[1,2],[0,3],[2,3],[0,1]]
for edge in edges:
    edge_1 = edge[0]
    edge_2 = edge[1]
    weight = 1.0
    G.add_edge(edge_1, edge_2,weight=weight)

rG = rx.PyGraph(multigraph=G.is_multigraph())
nodes = list(G.nodes)
node_indices = dict(zip(nodes, rG.add_nodes_from(nodes)))
rG.add_edges_from(
        [(node_indices[x[0]], node_indices[x[1]], x[2]['weight']) for x in G.edges(data=True)]
    ) 

general_lattice = Lattice(rG)