Qiskit / qiskit

Qiskit is an open-source SDK for working with quantum computers at the level of extended quantum circuits, operators, and primitives.
https://www.ibm.com/quantum/qiskit
Apache License 2.0
5.18k stars 2.35k forks source link

Optimization levels 2 and 3 fail when specifying a custom instruction with no properties #10604

Open caleb-johnson opened 1 year ago

caleb-johnson commented 1 year ago

Environment

What is happening?

I am adding a custom instruction to my backend target and then using generate_preset_pass_manager to create a pass manager with respect to the modified target. The custom instruction implements the _directive property and should just be ignored by the existing transpilation framework.

When I generate a level 2 or 3 pass manager, it errors out because it expects an iterable for the target.add_instruction(properties) argument; however, I do not need any properties to define my custom instruction and leave that argument to its default None value. Unless I misunderstand the docs, this should be the correct way to add this instruction.

How can we reproduce the issue?

First, install circuit-knitting-toolbox from source

from copy import deepcopy

from qiskit.circuit import QuantumCircuit
from qiskit.circuit.library.standard_gates import CXGate
from qiskit.providers.fake_provider import FakeLagosV2
from circuit_knitting_toolbox.circuit_cutting.qpd import SingleQubitQPDGate
from qiskit.transpiler.preset_passmanagers import generate_preset_pass_manager

# opt level 2 and 3 cause this script to fail
OPT_LEVEL = 2

backend = FakeLagosV2()
target = deepcopy(backend.target)
sample_qpd_instruction = SingleQubitQPDGate(QPDBasis.from_gate(CXGate()), 1)
target.add_instruction(sample_qpd_instruction)

qc = QuantumCircuit(1)
qc.append(sample_qpd_instruction, (0,))

pass_manager = generate_preset_pass_manager(OPT_LEVEL, target=target)
pass_manager.run(qc)
---------------------------------------------------------------------------
TypeError                                 Traceback (most recent call last)
Cell In[17], line 23
     21 pass_manager = generate_preset_pass_manager(OPT_LEVEL, target=target)
     22 #pass_manager.run(qpd_circuit)
---> 23 pass_manager.run(qc)

File ~/opt/anaconda3/envs/ckt/lib/python3.8/site-packages/qiskit/transpiler/passmanager.py:537, in StagedPassManager.run(self, circuits, output_name, callback)
    530 def run(
    531     self,
    532     circuits: _CircuitsT,
    533     output_name: Optional[str] = None,
    534     callback: Optional[Callable] = None,
    535 ) -> _CircuitsT:
    536     self._update_passmanager()
--> 537     return super().run(circuits, output_name, callback)

File ~/opt/anaconda3/envs/ckt/lib/python3.8/site-packages/qiskit/transpiler/passmanager.py:231, in PassManager.run(self, circuits, output_name, callback)
    229     return circuits
    230 if isinstance(circuits, QuantumCircuit):
--> 231     return self._run_single_circuit(circuits, output_name, callback)
    232 if len(circuits) == 1:
    233     return [self._run_single_circuit(circuits[0], output_name, callback)]

File ~/opt/anaconda3/envs/ckt/lib/python3.8/site-packages/qiskit/transpiler/passmanager.py:292, in PassManager._run_single_circuit(self, circuit, output_name, callback)
    280 """Run all the passes on a ``circuit``.
    281 
    282 Args:
   (...)
    289     The transformed circuit.
    290 """
    291 running_passmanager = self._create_running_passmanager()
--> 292 result = running_passmanager.run(circuit, output_name=output_name, callback=callback)
    293 self.property_set = running_passmanager.property_set
    294 return result

File ~/opt/anaconda3/envs/ckt/lib/python3.8/site-packages/qiskit/transpiler/runningpassmanager.py:125, in RunningPassManager.run(***failed resolving arguments***)
    123 for passset in self.working_list:
    124     for pass_ in passset:
--> 125         dag = self._do_pass(pass_, dag, passset.options)
    127 circuit = dag_to_circuit(dag, copy_operations=False)
    128 if output_name:

File ~/opt/anaconda3/envs/ckt/lib/python3.8/site-packages/qiskit/transpiler/runningpassmanager.py:173, in RunningPassManager._do_pass(self, pass_, dag, options)
    171 # Run the pass itself, if not already run
    172 if pass_ not in self.valid_passes:
--> 173     dag = self._run_this_pass(pass_, dag)
    175     # update the valid_passes property
    176     self._update_valid_passes(pass_)

File ~/opt/anaconda3/envs/ckt/lib/python3.8/site-packages/qiskit/transpiler/runningpassmanager.py:227, in RunningPassManager._run_this_pass(self, pass_, dag)
    224 elif pass_.is_analysis_pass:
    225     # Measure time if we have a callback or logging set
    226     start_time = time()
--> 227     pass_.run(FencedDAGCircuit(dag))
    228     end_time = time()
    229     run_time = end_time - start_time

File ~/opt/anaconda3/envs/ckt/lib/python3.8/site-packages/qiskit/transpiler/passes/layout/vf2_layout.py:148, in VF2Layout.run(self, dag)
    145 # Filter qubits without any supported operations. If they don't support any operations
    146 # They're not valid for layout selection
    147 if self.target is not None:
--> 148     has_operations = set(itertools.chain.from_iterable(self.target.qargs))
    149     to_remove = set(range(len(cm_nodes))).difference(has_operations)
    150     if to_remove:

TypeError: 'NoneType' object is not iterable

What should happen?

I believe a pass manager should be successfully generated for all optimization levels. I see no reason optimization levels 0 and 1 would accept None in add_instruction(properties) and optimization levels 2 and 3 would error out ungracefully.

Any suggestions?

No response

jakelishman commented 1 year ago

I haven't immediately looked at the bug, and this comment isn't related at all, but just beware: the CircuitInstruction you construct in your example is invalid. Because you pass it through QuantumCircuit.append, it probably gets normalised to the correct thing, but it's an internal object, so if you actually need to be using it, you need to be maintaining its invariants yourself. In this case, the types are bust: the qubits argument should be tuple[Qubit, ...], not tuple[int, ...]. That said, you don't need CircuitInstruction here at all: the call you actually mean is qc.append(sample_qpd_instruction, (0,)).

caleb-johnson commented 1 year ago

whoops, thanks I whipped up this little snippet quick. I just verified, and this doesn't change behavior

Updating original code snippet to correct usage

jakelishman commented 1 year ago

For the actual problem: looks like a bit of a bug in the VF2Layout handling that was added quickly to support backends with failing links; it should probably recognise that Target.qargs being None or containing None means that there's an operation that acts on all qubits. I'd have to look into the details of exactly how we intend to handle that in VF2PostLayout, which is working fine, since I remember it being relatively subtle since there are many reasons a target might include perfect operations on all qargs.

As an immediate workaround, you can be slightly more explicit and set the properties as

target.add_instruction(op, {(q,): None for q in range(target.num_qubits)})
caleb-johnson commented 1 year ago

it should probably recognise that Target.qargs being None or containing None means that there's an operation that acts on all qubits

I think I understand what you're saying, but in our case the gate doesn't act on all qubits -- it only acts on 2 qubits, but we want the transpiler to ignore that fact with respect to SWAP routing (and probably everything else). Treat it like a non-adjacent two-qubit barrier. Is that possible?

caleb-johnson commented 1 year ago

To be more concrete:

It seems like a circuit cutting transpilation routine would take an input circuit and produce an output circuit with N non-local gates flagged for cutting (based on some heuristic) with TwoQubitQPDGates. The layout stage will add these gates, but they should basically be ignored from then on. They are just directives to be used by a post-processing routine (circuit cutting).

jakelishman commented 1 year ago

Oh my bad sorry, I thought it was a 1q gate because I just read the SingleQubitQPDGate and missed that it was derived from a cx.

I think what you're currently doing is correct for what you want, it's just a bug in VF2Layout right now that prevents it from working. @mtreinish can check me on that, though.

caleb-johnson commented 1 year ago

Oh my bad sorry, I thought it was a 1q gate because I just read the SingleQubitQPDGate and missed that it was derived from a cx.

Hah, I forgot I used SingleQubitQPDGate in example. Either way, I believe we have a way forward :). Thanks a lot!

mtreinish commented 1 year ago

Yeah, the bug is definitely in VF2Layout and also in VF2PostLayout. The code which is computing the set of qubits which have instructions on them is not accounting for the value of the qargs being None for an ideal global instruction. This should be a simple fix to adjust https://github.com/Qiskit/qiskit-terra/blob/main/qiskit/transpiler/passes/layout/vf2_layout.py#L148 and https://github.com/Qiskit/qiskit-terra/blob/main/qiskit/transpiler/passes/layout/vf2_post_layout.py#L225 to skip a None entry in target.qargs (or maybe we shouldn't be putting None in target.qargs).

As a short term workaround you can try target.add_instruction(SingleQubitQPDGate, name=gate_name) which will treat SingleQubitQPDGate as a variadic instruction and the special handling for that may work.

mtreinish commented 1 year ago

@caleb-johnson I pushed up https://github.com/Qiskit/qiskit-terra/pull/10633 to fix this, if you wanted to give it a try and confirm it fixes the issue for you

caleb-johnson commented 11 months ago

Fixed by #10633

jakelishman commented 11 months ago

We can actually leave the issue open so others can see it's not merged yet, and mark #10633 as closing it.