Fix reverse permutation for transpiled circuits in Operator.from_circ…

…uit (Qiskit#8802)

* Fix reverse permutation for transpiled circuits in Operator.from_circuit

This commit fixes a bug in the Operator.from_circuit() constructor
method for Layout's generated by transpile(). The transpiler would set
the _layout property for an output circuit to be the mapping from the
input circuit's virtual qubits to the physical qubits (also the output
circuit's qubit index). This is useful for visualization and visually
tracking the permutation assuming you have the original circuit. However
for the `Operator.from_circuit()` constructor method which will reverse
the permutation caused by layout in the generated matrix this is not
sufficient information since we need to order of the original circuits
qubits. To fix this issue this commit changes the `_layout` attribute of
the QuantumCircuit class to store both the initial layout as before and
also a mapping of qubit objects to indices from the original circuit.
Using this extra information we can reliably handle the qubit
permutation in the constructor.

Fixes Qiskit#8800

* Update bip mapper test for new _layout format

Didn't see this locally because cplex is not installed.

* Remove stray debug prints

* Use a dataclass for _layout attribute

This commit updates the _layout QuantumCircuit attribute to store a new
dataclass, TranspileLayout, which currently stores the initial_layout
used by the transpiler and the initial qubit mapping which maps the
qubit object to it's position index in the circuit. For 0.23.0 we plan
to make the circuit layout attribute public and having a proper
container class will make it easier to do this in a way where we can
evolve the interface over time if needed (likely adding a final_layout
attribute in the near future). Right now the class isn't advertised or
re-exported as the interface is still private/internal but we can make
it a public interface at the same time we add a public getter for the
circuit layout.

* Disable bogus runtime import cyclic-import pylint error

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>

qiskit/compiler/transpiler.py

@@ -544,7 +544,7 @@ def _remap_circuit_faulty_backend(circuit, num_qubits, backend_prop, faulty_qubi
 
     for real_qubit in range(num_qubits):
         if faulty_qubits_map[real_qubit] is not None:
-            new_layout[real_qubit] = circuit._layout[faulty_qubits_map[real_qubit]]
+            new_layout[real_qubit] = circuit._layout.initial_layout[faulty_qubits_map[real_qubit]]
         else:
             if real_qubit in faulty_qubits:
                 new_layout[real_qubit] = faulty_qreg[faulty_qubit]

qiskit/quantum_info/operators/operator.py

@@ -200,9 +200,9 @@ def from_label(cls, label):
 
     @classmethod
     def from_circuit(cls, circuit, ignore_set_layout=False, layout=None):
-        """Create a new Operator object from a :class`.QuantumCircuit`
+        """Create a new Operator object from a :class:`.QuantumCircuit`
 
-        While a :class:`.QuantumCircuit` object can passed directly as ``data``
+        While a :class:`~.QuantumCircuit` object can passed directly as ``data``
         to the class constructor this provides no options on how the circuit
         is used to create an :class:`.Operator`. This constructor method lets
         you control how the :class:`.Operator` is created so it can be adjusted
@@ -221,7 +221,9 @@ def from_circuit(cls, circuit, ignore_set_layout=False, layout=None):
             layout (Layout): If specified this kwarg can be used to specify a
                 particular layout to use to permute the qubits in the created
                 :class:`.Operator`. If this is specified it will be used instead
-                of a layout contained in the ``circuit`` input.
+                of a layout contained in the ``circuit`` input. If specified
+                the virtual bits in the :class:`~.Layout` must be present in the
+                ``circuit`` input.
         Returns:
             Operator: An operator representing the input circuit
         """
@@ -230,15 +232,23 @@ def from_circuit(cls, circuit, ignore_set_layout=False, layout=None):
         if layout is None:
             if not ignore_set_layout:
                 layout = getattr(circuit, "_layout", None)
+        else:
+            from qiskit.transpiler.layout import TranspileLayout  # pylint: disable=cyclic-import
+
+            layout = TranspileLayout(
+                initial_layout=layout,
+                input_qubit_mapping={qubit: index for index, qubit in enumerate(circuit.qubits)},
+            )
         qargs = None
         # If there was a layout specified (either from the circuit
         # or via user input) use that to set qargs to permute qubits
         # based on that layout
         if layout is not None:
-            qargs = {
-                phys: circuit.find_bit(bit).index
-                for phys, bit in layout.get_physical_bits().items()
-            }
+            physical_to_virtual = layout.initial_layout.get_physical_bits()
+            qargs = [
+                layout.input_qubit_mapping[physical_to_virtual[physical_bit]]
+                for physical_bit in range(len(physical_to_virtual))
+            ]
         # Convert circuit to an instruction
         instruction = circuit.to_instruction()
         op._append_instruction(instruction, qargs=qargs)

qiskit/transpiler/basepasses.py

@@ -16,6 +16,7 @@
 from collections.abc import Hashable
 from inspect import signature
 from .propertyset import PropertySet
+from .layout import TranspileLayout
 
 
 class MetaPass(type):
@@ -131,7 +132,10 @@ def __call__(self, circuit, property_set=None):
             result_circuit = circuit.copy()
 
         if self.property_set["layout"]:
-            result_circuit._layout = self.property_set["layout"]
+            result_circuit._layout = TranspileLayout(
+                initial_layout=self.property_set["layout"],
+                input_qubit_mapping=self.property_set["original_qubit_indices"],
+            )
         if self.property_set["clbit_write_latency"] is not None:
             result_circuit._clbit_write_latency = self.property_set["clbit_write_latency"]
         if self.property_set["conditional_latency"] is not None:

qiskit/transpiler/layout.py

@@ -18,6 +18,9 @@
 Physical (qu)bits are integers.
 """
 
+from dataclasses import dataclass
+from typing import Dict
+
 from qiskit.circuit.quantumregister import Qubit, QuantumRegister
 from qiskit.transpiler.exceptions import LayoutError
 from qiskit.converters import isinstanceint
@@ -363,3 +366,11 @@ def from_qubit_list(qubit_list, *qregs):
         for qreg in qregs:
             out.add_register(qreg)
         return out
+
+
+@dataclass
+class TranspileLayout:
+    """Layout attributes from output circuit from transpiler."""
+
+    initial_layout: Layout
+    input_qubit_mapping: Dict[Qubit, int]

qiskit/transpiler/passes/layout/apply_layout.py

@@ -66,6 +66,9 @@ def run(self, dag):
         for creg in dag.cregs.values():
             new_dag.add_creg(creg)
         if post_layout is None:
+            self.property_set["original_qubit_indices"] = {
+                bit: index for index, bit in enumerate(dag.qubits)
+            }
             for qreg in dag.qregs.values():
                 self.property_set["layout"].add_register(qreg)
             virtual_phsyical_map = layout.get_virtual_bits()

qiskit/transpiler/runningpassmanager.py

@@ -24,6 +24,7 @@
 from .propertyset import PropertySet
 from .fencedobjs import FencedPropertySet, FencedDAGCircuit
 from .exceptions import TranspilerError
+from .layout import TranspileLayout
 
 logger = logging.getLogger(__name__)
 
@@ -128,7 +129,11 @@ def run(self, circuit, output_name=None, callback=None):
             circuit.name = output_name
         else:
             circuit.name = name
-        circuit._layout = self.property_set["layout"]
+        if self.property_set["layout"] is not None:
+            circuit._layout = TranspileLayout(
+                initial_layout=self.property_set["layout"],
+                input_qubit_mapping=self.property_set["original_qubit_indices"],
+            )
         circuit._clbit_write_latency = self.property_set["clbit_write_latency"]
         circuit._conditional_latency = self.property_set["conditional_latency"]
 

qiskit/visualization/circuit/latex.py

@@ -175,7 +175,10 @@ def __init__(
         self._plot_barriers = plot_barriers
         self._reverse_bits = reverse_bits
         if with_layout:
-            self._layout = self._circuit._layout
+            if self._circuit._layout:
+                self._layout = self._circuit._layout.initial_layout
+            else:
+                self._layout = None
         else:
             self._layout = None
 

qiskit/visualization/circuit/matplotlib.py

@@ -169,7 +169,10 @@ def __init__(
         self._plot_barriers = plot_barriers
         self._reverse_bits = reverse_bits
         if with_layout:
-            self._layout = self._circuit._layout
+            if self._circuit._layout:
+                self._layout = self._circuit._layout.initial_layout
+            else:
+                self._layout = None
         else:
             self._layout = None
 

qiskit/visualization/circuit/text.py

@@ -670,7 +670,10 @@ def __init__(
         self.clbits = clbits
         self.nodes = nodes
         if with_layout:
-            self.layout = self._circuit._layout
+            if self._circuit._layout:
+                self.layout = self._circuit._layout.initial_layout
+            else:
+                self.layout = None
         else:
             self.layout = None
 

qiskit/visualization/gate_map.py

@@ -812,22 +812,22 @@ def plot_circuit_layout(circuit, backend, view="virtual", qubit_coordinates=None
 
     bit_locations = {
         bit: {"register": register, "index": index}
-        for register in circuit._layout.get_registers()
+        for register in circuit._layout.initial_layout.get_registers()
         for index, bit in enumerate(register)
     }
-    for index, qubit in enumerate(circuit._layout.get_virtual_bits()):
+    for index, qubit in enumerate(circuit._layout.initial_layout.get_virtual_bits()):
         if qubit not in bit_locations:
             bit_locations[qubit] = {"register": None, "index": index}
 
     if view == "virtual":
-        for key, val in circuit._layout.get_virtual_bits().items():
+        for key, val in circuit._layout.initial_layout.get_virtual_bits().items():
             bit_register = bit_locations[key]["register"]
             if bit_register is None or bit_register.name != "ancilla":
                 qubits.append(val)
                 qubit_labels[val] = bit_locations[key]["index"]
 
     elif view == "physical":
-        for key, val in circuit._layout.get_physical_bits().items():
+        for key, val in circuit._layout.initial_layout.get_physical_bits().items():
             bit_register = bit_locations[val]["register"]
             if bit_register is None or bit_register.name != "ancilla":
                 qubits.append(key)

releasenotes/notes/fix-Opertor.from_circuit-transpile-5c056968ee40025e.yaml

@@ -0,0 +1,27 @@
+---
+upgrade:
+  - |
+    The ``._layout`` attribute of the :class:`~.QuantumCircuit` object has
+    been changed from storing a :class:`~.Layout` object to storing a
+    data class with 2 attributes, ``initial_layout`` which contains a
+    :class:`~.Layout` object for the initial layout set during compilation
+    and ``input_qubit_mapping`` which contains a dictionary mapping qubits
+    to position indices in the original circuit. This change was necessary to
+    provide all the information for a post-transpiled circuit to be able to
+    fully reverse the permutation caused by initial layout in all situations. While
+    this attribute is private and shouldn't be used externally, it is
+    the only way to track the initial layout through :func:`~.transpile`
+    so the change is being documented in case you're relying on it. If
+    you have a use case for the ``_layout`` attribute that is not being
+    addressed by the Qiskit API please open an issue so we can address this
+    feature gap.
+fixes:
+  - |
+    The :meth:`.Operator.from_circuit` constructor method has been updated
+    so that it can handle the layout output from :func:`~.transpile` and
+    correctly reverse the qubit permutation caused by layout in all cases.
+    Previously, if your transpiled circuit used loose :class:`~.Qubit` objects,
+    multiple :class:`~.QuantumRegister` objects, or a single
+    :class:`~.QuantumRegister` with a name other than ``"q"`` the constructor
+    would have failed to create an :class:`~.Operator` from the circuit.
+    Fixed `#8800 <https://github.com/Qiskit/qiskit-terra/issues/8800>`__.