Circuit drawer is illegible for instructions with circuit parameters.

[chriseclectic]

Environment

• Qiskit Terra version: 0.23.3
• Python version: 3.7
• Operating system: MacOS

What is happening?

If I try and make an instruction containing a quantum circuit as a parameter, the circuit drawer attempts to draw the contained circuit as the parameter value leading to an illegible drawing. Examples of such instructions are ControlFlowOps, thought these are treated as a special case in the drawer code.

How can we reproduce the issue?

from qiskit import QuantumCircuit, transpile
from qiskit.circuit import QuantumCircuit, Instruction
from qiskit.circuit.random import random_circuit

qc = random_circuit(2, 2)

inst = Instruction("block", 2, 0, [qc])
qc = QuantumCircuit(2)
qc.append(inst, range(2))
qc.draw(fold=-1)

shows

     ┌───────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────┐
q_0: ┤0                                                                                                                                                                                                                                              ├
     │  block(          ┌─────────────────────────────────┐
q_0: ──■──┤ U(0.24603,2.9981,1.8017,3.0896) ├
     ┌─┴─┐└────────────────┬────────────────┘
q_1: ┤ H ├─────────────────■─────────────────
     └───┘                                   ) │
q_1: ┤1                                                                                                                              │                                                                                                               ├
     └───────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────┼───────────────────────────────────────────────────────────────────────────────────────────────────────────────┘

What should happen?

Skip attempting to add parameter values to the drawing if parameters are circuits. This would also remove requiring a special case in the drawer code for ControlFlowOps, as they would just be handled by the general case of circuit parameters.

Eg, the above example should draw as

     ┌────────┐
q_0: ┤0       ├
     │  block │
q_1: ┤1       ├
     └────────┘

Any suggestions?

Change the get_param_str method to also skip QuantumCircuit parameters (it currenlty just skips ndarray parameters).

[jakelishman]

I agree that this isn't great, but what's the use-case for putting a QuantumCircuit in params? That's not really what params are ideally meant to be for (I wasn't a fan of us overloading it in ControlFlowOp either).

edit: fwiw, I think it might be cleaner if we have a list of parameter types that we allow drawing rather than a list that we skip, but it would be tricky to ensure that we don't introduce regressions if we do that.

[chriseclectic]

I am using it for intermediate instructions ins some complicated custom transpilation where i want to store a circuit block in an instruction for use later, and where i might want to transpile/modify that block while it is in the instruction in subsequent passes.

I tried doing this with a ControlFlowOp subclass but that doesnt work because DAGCircuit hard codes allowed ControlFlowOp subclasses. So it was easier to just make a regular Instruction subclass, but that leads to the current issue.

I like the idea of having allowed param values for printing though so they only get printed if numeric (int/float/complex) or string, and disable casting via str(obj) for other types

[jakelishman]

Ok yeah, that sounds fair as a current limitation. I'm interested in expanding the allowed "block" constructs in the future to include a nestable "basic block", but right now it's not a top priority and if your custom instruction just about works then that's ok.

For the allowed parameter types: off the top of my head, the list of types we'd need to accept would be at least instances of numbers.Number (covers Python builtins and Numpy types, and the speed loss isn't important here) and ParameterExpression, but it's probably worth us auditing the current instructions defined by Terra to see if there are any more.

An alternative strategy (that I'm not sure how I feel about) would be to attempt to str everything, and if the output has more than one line in it, it's skipped. There's less risk of regression from that, but equally, there's probably a good chance it doesn't solve the problem much better than the current situation.

[diemilio]

Hi @jakelishman. can I help with this?

[jakelishman]

Hi Diego, yeah absolutely, thanks! I'll assign you, and feel free to ask questions.

[diemilio]

Thanks @jakelishman, I created a PR with a fix for this specific issue, but let me know if you would like to add any of the other items you described above.

[jakelishman]

Sorry I didn't immediately respond here - I saw that you'd had a review on the other PR that you were going to do something with, so I'd left that. The direction you're going on that PR makes sense to me.