Closed nonhermitian closed 8 months ago
Did this work with a previous version of Qiskit with generate_preset_pass_manager
? If so, which version?
I thought we had an open issue about unifying the behaviour of transpile
and generate_preset_pass_manager
, but apparently not. There's a lot of rough edges right now, most of which were somewhat necessary for internal use when transpile
broadcast its arguments, but since that was removed, now it's (imo) just an interface bug when the two don't align, like here.
This isn't an interface bug about the people running with backends/targets and basis_gates. I expect this is a reused state bug, looking at this locally it runs the loop ~10 times and then fails when N == 12
because apparently the embedding step is not run in the pass manager. My assumption is that because the property set is getting reused between executions of pm
some state is skipping apply layout. My assumption is it's because of N<12 having a vf2layout match and when we hit 12 there isn't one anymore it's triggering sabreswap to run and it was skipping the appropriate layout steps. But I need to confirm
Either way if my guess is correct, then the correct thing to do is clear out the property set when run() is called.
I also can confirm that this exact script didn't fail in Qiskit 0.44.3. But it does fail with 1.0.0rc1
Yeah, I wasn't thinking it was the same as Target
/basis_gates
- I was thinking it was going to be more related to discussions that have happened offline where we realised that generate_preset_pass_manager
handles scheduling_method
and timing_constraints
differently to transpile
.
Ah yeah, that does make sense, and it could easily explain it if the bug appeared in 0.45 - that's when the pass manager handling changed, and the way the task iterator is generated changed between those versions. The problem is that PassManager
generates its property_set
on __init__
(like RunningPassManager
used to), not in _run_workflow
, which is where it should do it do be consistent with the old form. Should be a straightforwards bugfix and backportable to 0.46.
Yeah, I was using whatever was available before summit. I think the RC of 0.45
This issue actually pops up an interesting question to me: PassManager.property_set
has always fundamentally meant PassManager.run
can't be re-entrant, which isn't naively isn't much of a problem because we wouldn't expect people to use threaded parallelism over it for all the standard Python reasons. However, for the same reason, the parallel-dispatch form of PassManager.run
currently has no way of returning the property sets from the separate invocations, and there have been use-cases where we want to access analysis from within the transpilation after the run (I think some IBM-internal projects would like this, in particular).
Perhaps we ought to consider an expansion of the interface that allows returning both the final programs and the workflow states at some point? That would be a separate issue to this, of course.
Also, presumably the reason that things fail at 12 qubits in this particular set up is because Kyiv is heavy-hex, the input circuit is a ring graph, and ring(12)
is the first time that the input coupling dependency is and induced subgraph isomorphism of the Kyiv coupling constraints, so we take a different path through the layout stage (VF2Layout
rather than SabreLayout
), and that has an effect on the rest.
That's unrelated to the actual bug reported here, though it might be interesting to see why the embedding is failing if VF2Layout
succeeds on re-runs when everything's fine if it re-uses Sabre. Possibly VF2Layout
or some other component of our preset pipelines is more fragile in a way that could be a bug waiting to happen in a different form.
My suspicion (although I haven't had time to confirm it yet) is that it's actually an interaction of VF2PostLayout
and VF2Layout
when apply_layout
gets called it will have two layouts one for 12 qubits from vf2layout and one from vf2postlayout in the previous run and that is confusing things. The conditional logic around how we choose to skip passes is pretty intricate and makes a lot of assumptions about the workflow based on what gets left in the property set, so I could see being really fragile if you have a mix of leftover pieces in the property set from the previous runs
As long as it's all about the PropertySet
we're all good - that's state that should be explicitly be refreshed between full pass-manager invocations. My worry is that there's places where we have individual passes that store internal state within themselves that they don't refresh on calls to run
, and they cause problems on re-runs. Hopefully it's just all in the PropertySet
, though - that's what it should be.
As a short term workaround for anyone hitting something like this until #11787 is merged and released you can modify the example code to do something like:
import time
from qiskit.transpiler.preset_passmanagers import generate_preset_pass_manager
from qiskit.circuit.library import EfficientSU2
from qiskit_ibm_runtime import QiskitRuntimeService
from qiskit.passmanager import PropertySet
service = QiskitRuntimeService()
backend = service.get_backend('ibm_kyiv')
target = backend.target
pm = generate_preset_pass_manager(target=target, optimization_level=3)
qubits = list(range(2, 128))
qiskit_times = []
qiskit_depth = []
qiskit_2q = []
for N in qubits:
print('Starting', N, 'qubits')
qc = EfficientSU2(N, entanglement='circular')
print('circuit building done')
st = time.perf_counter()
trans_qc = pm.run(qc)
fin = time.perf_counter()
pm.property_set = PropertySet()
qiskit_times.append(fin-st)
qiskit_depth.append(trans_qc.depth())
qiskit_2q.append(trans_qc.count_ops().get('cx', 0))
print('Qiskit', fin-st, qiskit_2q[-1])
print()
Running pm.property_set = PropertySet()
will clear out the property set after running the pass manager so it's not getting reused between runs anymore.
Environment
What is happening?
Running benchmarks that were presented at Summit now error when using
generate_preset_pass_manager
.It fails at 12 qubits with the error:
TranspilerError: 'Fewer qubits in the circuit (12) than the coupling map (127). Have you run a layout pass and then expanded your DAG with ancillas? See
FullAncillaAllocation
,EnlargeWithAncilla
andApplyLayout
.'Oddly enough it works for smaller numbers .
How can we reproduce the issue?
What should happen?
It should work like it did in previous versions
Any suggestions?
No response