Transpiler effiency bug in synthesis

[epelofske-LANL]

Information

• Qiskit Terra version: qiskit-terra==0.18.2
• Python version: 3.9.7
• Operating system: CentOS Linux 7 (Core)

What is the current behavior?

/home/user/.conda/envs/stuff/lib/python3.9/site-packages/qiskit/transpiler/runningpassmanager.py:166: UserWarning: Resynthesized [<qiskit.dagcircuit.dagnode.DAGNode object at 0x2ae0e3027820>, <qiskit.dagcircuit.dagnode.DAGNode object at 0x2ae0e3aae8e0>] and got global phase: π/2, but the original was native 
and the new value is longer.  This indicates an efficiency bug in synthesis.  Please report it by opening an issue here: https://github.com/Qiskit/qiskit-terra/issues/new/choose
  new_dag = pass_.run(dag)

This circuit was also in the messsage:

Steps to reproduce the problem

I got this warning several times while using the qiskit transpiler. I am not able to retrieve the exact circuit that caused this error.

What is the expected behavior?

Suggested solutions

[ecpeterson]

That warning is less helpful than it could be, since DAGNodeOps don't have a useful __str__. That's on us. :/ I'll try to reproduce this, but it might be useful to replace that warning from Optimize1qGatesDecomposition.run with the following:

            warnings.warn(
                f"Resynthesized \n\n" + 
                "\n".join([str(node.op) for node in old_run]) +
                f"\n\nand got\n\n" +
                "\n".join([str(node[0]) for node in new_circ]) +
                f"\n\nbut the original was native and the new value is longer.  This "
                f"indicates an efficiency bug in synthesis.  Please report it by "
                f"opening an issue here: "
                f"https://github.com/Qiskit/qiskit-terra/issues/new/choose",
                stacklevel=2,
            )

[1ucian0]

@epelofske-LANL can you try again with, now that #7048 ? You will have to install qiskit from main tho. Your code to reproduce the warning (including the circuit and the transpile options) would also help otherwise.

[1ucian0]

#7048 will be included in 0.18.3. You can either install main or wait for that release (it might happen any time in the next two weeks)

[epelofske-LANL]

I installed from source, and managed to get this warning:

/home/user/.local/lib/python3.9/site-packages/qiskit/transpiler/passes/optimization/optimize_1q_decomposition.py:171: UserWarning: Resynthesized 

Instruction(name='sx', num_qubits=1, num_clbits=0, params=[])
Instruction(name='x', num_qubits=1, num_clbits=0, params=[])

and got

Instruction(name='rz', num_qubits=1, num_clbits=0, params=[-3.141592653589793])
Instruction(name='sx', num_qubits=1, num_clbits=0, params=[])
Instruction(name='rz', num_qubits=1, num_clbits=0, params=[-3.141592653589793])

but the original was native (for ['x', 'sx', 'cx', 'rz']) and the new value is longer.  This indicates an efficiency bug in synthesis.  Please report it by opening an issue here: https://github.com/Qiskit/qiskit-terra/issues/new/choose
  if new_circ is not None and self._substitution_checks(dag, run, new_circ, new_basis):

I am trying to save a circuit which gives this warning, and will post it here if I can replicate it (it seems to be hard to replicate).

[epelofske-LANL]

I found a circuit which gave the user warning (edit; the user warning was the same as the above post, and is the same as the original start to this issue):

circuit.txt

The transpiler call I made that produced the warning used several options, including specifying the coupling map of ibmq_guadalupe:

backend = provider.get_backend("ibmq_guadalupe")
config = backend.configuration()
hardware_connectivity = networkx.Graph(config.coupling_map)
compiled_circuit = transpile(qc, coupling_map=CouplingMap(list(hardware_connectivity.edges())), optimization_level=3, basis_gates=["x", "sx", "cx", "rz"], initial_layout=[8, 11, 12, 13, 14, 15])

[levbishop]

Thanks for reproducing. This shows a bug with the decision to incorrectly consider the new circuit as less efficient, therefore displaying the warning message. The new circuit uses only one sqrt(x) pulse whereas the old uses a sqrt(x) and an x pulse. So the new will actually be an improvement in terms of fidelity, but since it uses two phase gates the new uses more total gates and the length check incorrectly marks it as an inefficient synthesis. For this case you can ignore the warning. EDIT: the same check is used to see whether to emit the new or old sequence so if you are running on hardware (not a simulator) this bug would lead to a lower fidelity.

We need to change the checks to count pulses first and only consider comparing total gates if old circuit and new circuit have equal pulse count.