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
4.87k stars 2.3k forks source link

Flipping of gate direction in Transpiler for CZ gate #8387

Closed JMuff22 closed 1 year ago

JMuff22 commented 1 year ago

What should we add?

The current version of the transpiler raises the following error when attempting to set the basis gate of a backend as cz

'Flipping of gate direction is only supported for CX, ECR, and RZX at this time, not cz.'

As far as I'm aware and through my own testing flipping of a CZ should be easy to implement as it's "flip" produces the same unitary operator. There is no direct identity needed.

If this could get added that would be great! I would have done this via a pull request myself but it seems "too easy" of a solution that I must be missing something.

The Qiskit file that would need changing. https://github.com/Qiskit/qiskit-terra/blob/1312624309526812eb62b97e0d47699d46649a25/qiskit/transpiler/passes/utils/gate_direction.py

alexanderivrii commented 1 year ago

@JMuff22, you are correct, CZ is symmetric. Could you please provide a code snippet that results in the above error?

JMuff22 commented 1 year ago

Here is the code snippet. It is essentially the same as Qiskit Tenerife currently but with the change in the basis_gates.

import os
import json
from qiskit.test.mock.fake_backend import FakeBackend
from qiskit.providers.models import GateConfig, QasmBackendConfiguration, BackendProperties

class FakeTestBackend(FakeBackend):
    """A fake 5 qubit backend."""

    def __init__(self):
        """
        .. code-block:: text

        """
        cmap = [[1, 0], [2, 0], [2, 1], [3, 2], [3, 4], [4, 2]]
        """ https://qiskit.org/documentation/stubs/qiskit.providers.models.QasmBackendConfiguration.html """
        configuration = QasmBackendConfiguration(
            backend_name="fake_testbackend",
            backend_version="0.0.0",
            n_qubits=5,
            basis_gates=["r", "cz"],
            simulator=False,
            local=True,
            conditional=False,
            open_pulse=False,
            memory=False,
            max_shots=5000,
            max_experiments=100,
            gates=[GateConfig(name="TODO", parameters=[], qasm_def="TODO")],
            coupling_map=cmap,
        )

        super().__init__(configuration)

This is called for example via

from fake_testbackend import FakeTestBackend
from qiskit.providers.aer import AerSimulator
device_backend =FakeTestBackend()
sim_fakebackend = AerSimulator.from_backend(device_backend)

# Construct quantum circuit
circ = QuantumCircuit(3, 3)
circ.h(0)
circ.cx(0, 1)
circ.cx(1, 2)
circ.measure([0, 1, 2], [0, 1, 2])

tcirc = transpile(circ, sim_vigo)

# Execute noisy simulation and get counts
result_noise = sim_fakebackend.run(tcirc).result()
counts_noise = result_noise.get_counts(0)

print(counts_noise)

Also, forgot to mention it is probably important to note that I am deliberately running an older version of Qiskit due to vendor software being behind a bit. Hence the use of the now deprecated from qiskit.test.mock.fake_backend import FakeBackend.

> qiskit.__qiskit_version__
{'qiskit-terra': '0.19.2', 'qiskit-aer': '0.10.3', 'qiskit-ignis': '0.7.0', 'qiskit-ibmq-provider': '0.18.3', 'qiskit-aqua': None, 'qiskit': '0.34.2', 'qiskit-nature': None, 'qiskit-finance': None, 'qiskit-optimization': None, 'qiskit-machine-learning': None
JMuff22 commented 1 year ago

Any update on this?

alexanderivrii commented 1 year ago

@JMuff22, seems that no one picked up on this. Sure, modifying the code in gate_direction.py would work.

Alternatively, there was some earlier discussion in https://github.com/Qiskit/qiskit-terra/pull/7875#pullrequestreview-936996092 to annotate a gate type or instance as being "qarg_symmetric", which seems like a more general solution.

@1ucian0, @ajavadia, @kdk, what do you think is the best direction?

JMuff22 commented 1 year ago

I could submit a PR myself, something like:

                elif node.name == "cz":
                    if (physical_q0, physical_q1) in self.target["cz"]:
                        continue
                    if (physical_q1, physical_q0) in self.target["cz"]:
                        dag.substitute_node_with_dag(node, self._cz_dag(*node.op.params))
                    else:
                        raise TranspilerError(
                            "The circuit requires a connection between physical "
                            "qubits %s and %s for cz" % (physical_q0, physical_q1)
                        )

And

        self._cz_dag = DAGCircuit()
        qr = QuantumRegister(2)
        self._cx_dag.add_qreg(qr)
        self._cz_dag.apply_operation_back(CXGate(), [qr[1], qr[0]], [])

I am not that well versed is Qiskit's own style guide or even sure if the code above is too complicated to resolve the issue. Maybe just a flag before the else statement in gate_direction.py to catch symmetric gates and pass them along?

eendebakpt commented 1 year ago

@JMuff22 I encountered the same issue and took the liberty to create a PR based on your code.

JMuff22 commented 1 year ago

Great thanks for that! Hopefully it gets merged!

epelofske-LANL commented 1 year ago

I just got this error with some code I am testing: qiskit.transpiler.exceptions.TranspilerError: 'Flipping of gate direction is only supported for CX, ECR, and RZX at this time, not cz.'. My local version of qiskit terra is qiskit-terra==0.20.2; is the fix not in the most current version of terra?