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

Circuit drawers fail with bits as conditions #7284

Closed
enavarro51 opened this issue Nov 17, 2021 · 3 comments · Fixed by #7285
Closed

Circuit drawers fail with bits as conditions #7284

enavarro51 opened this issue Nov 17, 2021 · 3 comments · Fixed by #7285
Assignees
Labels
bug Something isn't working help wanted community contributions welcome. For filters like http://github-help-wanted.com/

Comments

@enavarro51
Copy link
Contributor

enavarro51 commented Nov 17, 2021

Environment

  • Qiskit Terra version: Current main
  • Python version: 3.8
  • Operating system: Ubuntu

What is happening?

The method _get_layered_instructions in qiskit.visualization.utils.py makes the assumption when placing gates into layers for display that all conditions and all measures are on registers only. Using bits will crash all 3 drawers as shown in the circuit below.

How can we reproduce the issue?

The following circuit

from qiskit.circuit import QuantumCircuit, Qubit, Clbit, ClassicalRegister

bits = [Qubit(), Qubit(), Clbit(), Clbit()]
cr = ClassicalRegister(2)
crx = ClassicalRegister(4)
qc = QuantumCircuit(bits, cr, [Clbit()], crx)
qc.x(0).c_if(cr[1], 0)
qc.measure(0, bits[2])
qc.draw('mpl')

Fails with,

~/qiskit/qiskit-terra/qiskit/visualization/circuit_visualization.py in _matplotlib_circuit_drawer(circuit, scale, filename, style, plot_barriers, reverse_bits, justify, idle_wires, with_layout, fold, ax, initial_state, cregbundle)
    648     """
    649 
--> 650     qubits, clbits, nodes = utils._get_layered_instructions(
    651         circuit, reverse_bits=reverse_bits, justify=justify, idle_wires=idle_wires
    652     )

~/qiskit/qiskit-terra/qiskit/visualization/utils.py in _get_layered_instructions(circuit, reverse_bits, justify, idle_wires)
    305             nodes.append([node])
    306     else:
--> 307         nodes = _LayerSpooler(dag, justify, measure_map, reverse_bits)
    308 
    309     if reverse_bits:

~/qiskit/qiskit-terra/qiskit/visualization/utils.py in __init__(self, dag, justification, measure_map, reverse_bits)
    386                 dag_nodes = _sorted_nodes(dag_layer)
    387                 for node in dag_nodes:
--> 388                     self.add(node, current_index)
    389         else:
    390             dag_layers = []

~/qiskit/qiskit-terra/qiskit/visualization/utils.py in add(self, node, index)
    508         """Add 'node' where it belongs, starting the try at 'index'."""
    509         if self.justification == "left":
--> 510             self.slide_from_left(node, index)
    511         else:
    512             self.slide_from_right(node, index)

~/qiskit/qiskit-terra/qiskit/visualization/utils.py in slide_from_left(self, node, index)
    417         measure_layer = None
    418         if isinstance(node.op, Measure):
--> 419             measure_reg = next(reg for reg in self.measure_map if node.cargs[0] in reg)
    420 
    421         if not self:

StopIteration: 

Other errors occur with differing circuits.

What should happen?

Code shouldn't crash.

Any suggestions?

This will require a fairly complete rewrite of the layering code for the drawers to account for both registers and bits being conditions and measure targets.

@jakelishman
Copy link
Member

Yeah, this seems like it's another manifestation of the old assumptions that registers were the primitive, and bits were purely an element of a register. That's the case in all the drawers for the purposes of picking labels as well:

In [1]: from qiskit.circuit import Qubit, QuantumRegister, QuantumCircuit
   ...: bits = [Qubit() for _ in [None]*3]
   ...: qc = QuantumCircuit(QuantumRegister(bits=bits[:2]), QuantumRegister(bits=bits[1:]))
   ...: qc.h(bits)
   ...: qc.draw()
Out[1]:
      ┌───┐
q0_0: ┤ H ├
      ├───┤
q1_0: ┤ H ├
      ├───┤
q1_1: ┤ H ├
      └───┘

I think we probably need to make sure we're all on the same page that registers are now just "a handle to some bits", and then perhaps look at the drawers (and plenty of other places!) with a high-level view that we need to change some assumptions. Thanks for finding and reporting this - it's good to try to stay on top of these things!

@enavarro51
Copy link
Contributor Author

Yeah, I feel like a lot of this transition has been a bit ad hoc. A high-level overview of this would be helpful with some clear guidance on how we want to present it qiskit-wide. #6761 is the next level that may not work in some hidden places.

Can you assign this one to me?

@jakelishman
Copy link
Member

Sure thing, thanks for all the work on this stuff!

@javabster javabster added the help wanted community contributions welcome. For filters like http://github-help-wanted.com/ label Nov 19, 2021
@mergify mergify bot closed this as completed in #7285 Nov 26, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working help wanted community contributions welcome. For filters like http://github-help-wanted.com/
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants