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

DAGs imported from networkx cannot be drawn #7915

Closed
m-malinowski opened this issue Apr 10, 2022 · 2 comments · Fixed by #7923
Closed

DAGs imported from networkx cannot be drawn #7915

m-malinowski opened this issue Apr 10, 2022 · 2 comments · Fixed by #7923
Labels
bug Something isn't working

Comments

@m-malinowski
Copy link

Informations

  • Qiskit version: 0.19.2
  • Python version: 3.8.12
  • Operating system: Windows 10

What is the current behavior?

I can successfully create DAGCircuit objects by importing them from networkx graphs. Those objects seem to behave correctly, but whenever I try to draw them using the dag_drawer function, I get an error. I verified that the error only occurs for DAGCircuit objects imported from networkx graphs - in fact, I can draw a dag correctly, but if I convert it to networkx and back (new_dag = from_networkx(dag.to_networkx())), then new_dag can no longer be drawn.

Steps to reproduce the problem

import qiskit
from qiskit.circuit.random import random_circuit
from qiskit.converters import circuit_to_dag, dag_to_circuit
from qiskit.tools.visualization import dag_drawer
from qiskit.dagcircuit import DAGCircuit

in_circ = random_circuit(1,1)
in_dag = circuit_to_dag(in_circ)
# dag_drawer(in_dag) # if uncommented, draws the DAG correctly
graph = in_dag.to_networkx()
out_dag = DAGCircuit.from_networkx(graph)
dag_drawer(out_dag) # throws an error

outputs

KeyError                                  Traceback (most recent call last)
---> [12]() dag_drawer(out_dag)


--> [224]() dot_str = dag._multi_graph.to_dot(node_attr_func, edge_attr_func, graph_attrs)

--> [206]()     n["label"] = bit_labels[node.wire]
 
KeyError: Qubit(QuantumRegister(1, 'q'), 0)

What is the expected behavior?

That DAG objects created from networkx graphs have the same properties as those created from quantum circuits.

Suggested solutions

@m-malinowski m-malinowski added the bug Something isn't working label Apr 10, 2022
@jakelishman jakelishman transferred this issue from Qiskit/qiskit-metapackage Apr 10, 2022
@mtreinish
Copy link
Member

Honestly, I'm thinking it's time that we deprecate and prepare to remove the networkx converters for the dagcircuit. We originally had them when we first switched to use retworkx because the api functionality was more limited. But, I'm not sure there is much value in having them anymore as retworkx can do a lot more than 2 years ago and we can quickly add any functionality its missing. Is there a particular function or use case you have for converting to networkx graphs?

That being said I think the issue is in the from_networkx has no context about registers and the dag_drawer only works with registers for edge labels and in and out node labels. The graph representation can't really know about any registers with the circuit generally because that's metadata from a circuit . It might be a good use case for a networkx graph attribute to store the registers list, but I think given the pending deprecation I mentioned above it's not worth doing that. The dag_drawer method needs to be updated to work with bits directly and not only work with registers. I'll push up a fix to do this shortly

mtreinish added a commit to mtreinish/qiskit-core that referenced this issue Apr 12, 2022
This commit fixes the handling of generating visualizations for
DAGCircuit objects that have standalone bit objects. Previously, the
dag_drawer() function was assuming every bit in the dag was a member of
a register. In the case of circuits with standalone bits this is not the
case and would result in a KeyError being raised when the register was
attempted to be used to generate a string label for the node or edge
in the visualization. This fixes this by still using registers if
present, but in the absence of a register it fallsback to using it's
absolute index in the dag to generate a similar label. For example, the
4th qubit will have the label "q[4]" in the output visualization.

Fixes Qiskit#7915
@m-malinowski
Copy link
Author

Hi. Yeah, I see where it's coming from. In my view, networkx is a handy tool for interacting with DAGs flexibly. I'm playing around with some custom transpilation methods, and as a beginner, using networkx means I can find a lot of examples / tutorials / discussion online. But I see where you're coming from, and you might not want to support this kind of usecases.

From my perspective, it would be most flexible to set things up such that if a DAGCircuit can be initialised without some metadata, then all in-built functions that operate on DAGCircuits don't crash even if the metadata is not provided. Alternatively, some metadata could be added by default if not explicitly specified?

@mergify mergify bot closed this as completed in #7923 Apr 14, 2022
mergify bot added a commit that referenced this issue Apr 14, 2022
* Fix dag visualization for DAGCircuits without registers

This commit fixes the handling of generating visualizations for
DAGCircuit objects that have standalone bit objects. Previously, the
dag_drawer() function was assuming every bit in the dag was a member of
a register. In the case of circuits with standalone bits this is not the
case and would result in a KeyError being raised when the register was
attempted to be used to generate a string label for the node or edge
in the visualization. This fixes this by still using registers if
present, but in the absence of a register it fallsback to using it's
absolute index in the dag to generate a similar label. For example, the
4th qubit will have the label "q[4]" in the output visualization.

Fixes #7915

* Adjust default label from q[0] to q_0

This commit changes the default fallback label when no register match is
found to be of the form q_0, where 0 is the qubit index. This was done
to avoid potential confusion with a register named q which might be in
the circuit and not the first element in the qubits list.

* Add release note

* Add test

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
mergify bot pushed a commit that referenced this issue Apr 14, 2022
* Fix dag visualization for DAGCircuits without registers

This commit fixes the handling of generating visualizations for
DAGCircuit objects that have standalone bit objects. Previously, the
dag_drawer() function was assuming every bit in the dag was a member of
a register. In the case of circuits with standalone bits this is not the
case and would result in a KeyError being raised when the register was
attempted to be used to generate a string label for the node or edge
in the visualization. This fixes this by still using registers if
present, but in the absence of a register it fallsback to using it's
absolute index in the dag to generate a similar label. For example, the
4th qubit will have the label "q[4]" in the output visualization.

Fixes #7915

* Adjust default label from q[0] to q_0

This commit changes the default fallback label when no register match is
found to be of the form q_0, where 0 is the qubit index. This was done
to avoid potential confusion with a register named q which might be in
the circuit and not the first element in the qubits list.

* Add release note

* Add test

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
(cherry picked from commit 4463ac3)
mergify bot added a commit that referenced this issue Apr 14, 2022
* Fix dag visualization for DAGCircuits without registers

This commit fixes the handling of generating visualizations for
DAGCircuit objects that have standalone bit objects. Previously, the
dag_drawer() function was assuming every bit in the dag was a member of
a register. In the case of circuits with standalone bits this is not the
case and would result in a KeyError being raised when the register was
attempted to be used to generate a string label for the node or edge
in the visualization. This fixes this by still using registers if
present, but in the absence of a register it fallsback to using it's
absolute index in the dag to generate a similar label. For example, the
4th qubit will have the label "q[4]" in the output visualization.

Fixes #7915

* Adjust default label from q[0] to q_0

This commit changes the default fallback label when no register match is
found to be of the form q_0, where 0 is the qubit index. This was done
to avoid potential confusion with a register named q which might be in
the circuit and not the first element in the qubits list.

* Add release note

* Add test

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
(cherry picked from commit 4463ac3)

Co-authored-by: Matthew Treinish <mtreinish@kortar.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants