Better drawings for control flow

[gabrieleagl]

What should we add?

Currently, if_else, while_loop and for_loop are represented in drawings as black boxes.
Additionally, the tested condition is not explicit in the drawing.

I propose adding a more explicit representation.
In the figure below I propose two alternatives, so that we can choose one or discuss an even better option.

[gabrieleagl]

@jakelishman as discussed on slack

[enavarro51]

@gabrieleagl I've thought about this a bit as well. I feel like your Idea 1 will work and provides the best option. For the mpl drawer, it might not require changing the wire spacing, and the text drawer already adjusts wire spacing to fit other text between gates.

I believe it would be possible to adapt the current _multiqubit_gate code in the mpl drawer to create the boxes. I also think it would be possible to recall _get_layered_instructions followed by creating a new instance of MatplotlibDrawer for the bodies of the ControlFlow methods. I was thinking of seeing if I can work up a basic demo this weekend.

The text and latex drawers may require a bit more work, since there is not as much flexibility with the spacing and fonts.

[enavarro51]

Here's a not-ready-for-primetime demo for the mpl drawer with an 'if'. This took a bit less than 50 lines of code, so no major restructuring of the drawer is required. This should work with any arbitrary circuit inside the 'if', and should also work with any of the other ControlFlowOps.

To-dos:

• Adjust the spacing of the gates inside the box.
• The bottom of the box should probably be just below the bottom H gate and the top of the line down to the clbit should probably start from the bottom of the 'if' box.
• Nested ControlFlowOps are not working yet, but should not be too difficult.
• I'm not sure what we want to do about the text, other than just 'IF' at the top.

true_body = QuantumCircuit(3)# when true case
true_body.h(true_body.qubits[1:3])
true_body.x(1)

qbit = QuantumRegister(3, "q")
cbit = ClassicalRegister(3, "c")
qc = QuantumCircuit(qbit, cbit)
qc.y(0)
qc.measure(0, 0)

op = IfElseOp((qc.clbits[0], True), true_body, None)
qc.append(op, [0, 1, 2], [0])

qc.draw()

[jakelishman]

Oh, this looks great as a start, thanks so much Edwin!

For your sanity: I think drawing the box around the clbits is the more sensible way to go - control-flow may well act on the bits that it's conditioned on, and it'd start getting really confusing trying to do something sensible if the block's condition was on a clbit that's above (in the drawing) another clbit it acts on. Instead, to show the condition better, perhaps we don't draw the condition at all (in the normal mechanism), and instead write some representation of it up above the blocks? I think that'll be easier to make things like paired "if" and "else" blocks look better - we could do something like write the condition, then have a big brace coming out from it to hold the two blocks, so it's clear they're from the same condition.

One more thing to consider: I think we'll still want a sensible way of showing qubits/clbits that are unaffected by the blocks, much like opaque instructions currently do in the drawers. We can probably do that by drawing some sigil where the input wires cross the block delimiter, or perhaps better, we could try to not draw the wires at all within the block?

[gabrieleagl]

Thanks Edwin!

I don't want to over-complicate, but the most clear option to me would be:

• drawing a vertical wire from the box to the clbits involved in the if_else condition
• drawing a vertical wire from the gates inside the box to the clbits in case of conditioned gates or measurements, as usual.

When we move from the if_else to the loops, setting filled vs empty circles on the vertical wire, is no longer an option to indicate if the qubit is tested to be 1 vs 0, as conditions may depend on the loop variables. So we can either plot the formula, or simply indicate the qubits and discard the formula.
For homogeneity, we can do the same for if_else, without putting filled/empty circles.

What do you think? Jake?

[jakelishman]

I'm keen to stick close to what Edwin thinks is reasonably implementable here, because we 100% do not have the resources to make Qiskit's visualisations "perfect". We've generally maintained the position that we would support a more full-featured visualisation package being responsible for the best drawings, and that Qiskit's internal support is just best-effort.

As a direct response: I think drawing the box around all the affected wires (both qubit and clbit), and not doing something different between qubits and clbits is the best way forwards. There are many edge cases, such as blocks that aren't on visually contiguous qubits/clbits, that are already considered for opaque instructions (i.e. we draw a box from the top-most qubit to bottom-most clbit and label unused wires), and I think it will be simplest to stick close to that type of approach. It will also mean that if a control-flow block contains one of these opaque instructions, we won't suddenly end up with the case where an opaque instruction is sticking out of the surrounding control-flow box.

I agree that writing out the condition / loop indices above the box(es) will probably be the neatest (and easier to implement).

[enavarro51]

@gabrieleagl I thought of something similar to your top drawing, but I think it's probably simpler and more easily readable if we just put the condition for the IfElseOp in text at the top. And for the loops that would work as well. More demos to follow.

[enavarro51]

Ok. Here's a first semi-working version of an If. The following circuit generates the drawing,

   1  qr = QuantumRegister(4, "qr")
   2  cr = ClassicalRegister(3, "cr")
   3  qc = QuantumCircuit(qr, cr)
   4  qc.h(0)
   5  qc.h(1)
   6  qc.measure(0, 1)
   7  qc.measure(1, 2)
   8  qc.x(2)
   9  qc.x(2, label="XLabel").c_if(cr, 2)
  10  
  11  qr2 = QuantumRegister(3, "qr2")
  12  qc2 = QuantumCircuit(qr2, cr)
  13  qc2.x(1)
  14  qc2.y(1)
  15  qc2.x(0)
  16  qc2.x(2).c_if(cr, 4)
  17  
  18  qc_ifop = IfElseOp((cr[1], 1), qc2, qc3)
  19  qc.append(qc_ifop, [0, 1, 2], [0])
  20  qc.x(0)
  21  
  22  qc.draw(style={"showindex": True})

If you change line 16 to qc2.x(0).c_if(cr, 4), you get

So the IfElseOp box scales to accommodate whatever circuit is inside.

Here's what's left for the If/Else,

1. Add the Else box when there is an else. This should be fairly straightforward - just appending a box onto the If with the Else circuit inside.
2. Deal with what happens when the drawing wraps to a new line and an If/Else will need to be part one line and part another. I have some ideas how to deal with this but open to suggestions.
3. Handle nested If's. I have structured the code to recursively handle nested If's, but that will need some debugging and multiple tests.
4. Finalize box shape and color. I picked these arbitrarily, but can be anything. I made the box edge thick and with a color so it doesn't get confused with the wires when there's a large circuit inside.

For the future, there are the ForLoopOp and WhileLoopOp. These come with some new issues.

The future plan is

1. Do an initial PR just for the IfElseOp with no nested If/Else's. Any If/Else inside another If/Else will initially display just as a multi-qubit gate like it does now. That will allow finalizing the basic code before adding the recursion.
2. Do the recursion in a second PR with added tests.
3. Take on the For and While in additional PR(s).
4. Add all this functionality to the text drawer. At the moment, absolutely no idea how to do it.

[jakelishman]

This looks great, thanks Edwin. For how to handle wrapping: my initial reaction is that we could just count these boxes as "atomic", and force them to start a new line if necessary. If they're still too long for the line length, then we could just extend the line - I think it'll be pretty hard to read if we split the boxes. I'm completely open to whatever you've got in mind too.

I don't have preferences on box shape or colour, provided that the colour is distinguishable, controllable in user stylesheets, and for our default style probably just uses an existing colour. Your picture looks like it matches all those points.

In your "box scaling": your box seems to scale horizontally but not vertically. Is that just because you built the IfElseOp manually, and it explicitly blocks on qr[2] even though it's not in use? Good if so - that looks like expected behaviour. For a full set of tests, you might want to use the control-flow builder interface to construct things, since it's the way people will generally build up their circuits, and it should be easier to work with. For example, your code would look like:

qr = QuantumRegister(4, "qr")
cr = ClassicalRegister(3, "cr")
qc = QuantumCircuit(qr, cr)
qc.h(0)
qc.h(1)
qc.measure(0, 1)
qc.measure(1, 2)
qc.x(2)
qc.x(2, label="XLabel").c_if(cr, 2)
with qc.if_test((cr[1], 1)):
    qc.x(1)
    qc.y(1)
    qc.x(0)
    qc.x(2).c_if(cr, 4)
qc.x(0)

qc.draw(style={"showindex": True})

Your plan sounds sensible to me.

[gabrieleagl]

Great job @enavarro51 - I love it!

For the problem of breaking lines, another option could be to make two boxes on the two lines, and finding a way to mark that it's a continuation (such as a double or dashed line on the right side of the box on the first line, and a similar double or dashed on the left of the second line).

One possibly hard test case, that you didn't mention: a gate that is both quantum-controlled and classically-conditioned.

[enavarro51]

Ok, Team. I now have nested if/else working down to whatever level. There are still some spacing issues, but now this circuit,

qr = QuantumRegister(4, "q")
cr = ClassicalRegister(3, "cr")
qc = QuantumCircuit(qr, cr)
qc.h(0)
qc.h(1)
qc.measure(0, 1)
qc.measure(1, 2)
qc.x(2)
with qc.if_test((cr[1], 1)) as _else:
    qc.y(1)
    qc.y(1)
    qc.x(0, label="X111i")
    qc.x(0, label="X111i").c_if(cr, 4)
    with qc.if_test((cr[0], 1)):
        qc.z(0)
with _else:
    qc.x(0, label="Z111i")
    qc.y(0)
    qc.y(0)
    qc.y(2)
    with qc.if_test((cr[2], 1)):
        qc.x(1)
        qc.x(2)
qc.x(0)

qc.draw(style={"showindex": True}, fold=50, scale=0.8)

generates this in main,

and this with the new stuff,

Still more to do before a PR, but progress.

[enavarro51]

So this crazy circuit,

qr = QuantumRegister(4, "q")
cr = ClassicalRegister(3, "cr")
qc = QuantumCircuit(qr, cr)
qc.h(0)

with qc.if_test((cr[1], 1)) as _else:

    qc.x(0, label="X1111i").c_if(cr, 4)
    with qc.if_test((cr[2], 1)):
        qc.z(0)
        with qc.if_test((cr[1], 1)):
            qc.cx(0, 1)
            with qc.if_test((cr[2], 1)):
                qc.x(1)
                qc.x(2)
                with qc.if_test((cr[1], 1)):
                    qc.cx(0, 1)
with _else:
    qr1 = QuantumRegister(2, "qr1")
    cr1 = ClassicalRegister(2, "cr1")
    cr2 = ClassicalRegister(2, "cr2")
    circuit = QuantumCircuit(qr1, cr1, cr2)
    inst = QuantumCircuit(2, 2, name="Inst").to_instruction()
    qc.append(inst, [qr[0], qr[1]], [cr[0], cr[1]])

    qc.y(1)
    with qc.if_test((cr[2], 1)):
        qc.x(1)
        qc.x(2)
qc.x(0)


qc.draw(style={"showindex": True})

produces this crazy drawing

The first if is nested 5 deep. There is no limit to the depth. The colors cycle through a group of 4 (thank you @jlapeyre for the idea).

The plan is now to do 2 PRs. The first will be a number of structural data changes that alter 200 lines of the mpl drawer code, but should function exactly as the current main. This was discussed with @jakelishman and is being done to simplify the review process, since these changes are almost all simple like changing self._data to node_data in many places.

The second will be a WIP PR for the actual if/else functionality. The big missing piece right now, other than more testing, is how to wrap the if/else boxes.

[1ucian0]

I think this was closed via #10207 Let me know otherwise.