Skip decomposer construction in consolidate blocks with force_consolidate

In the recently merged Qiskit#16075 a default decomposer was added to the
consolidate block pass if the `force_consolidate` flag was set. This was
intended to enable running the pass in the absence of the options
required to configure a decomposer. If the force consolidate flag is set
we don't need a two qubit decomposer configured since we're always going
to consolidate a 2q block of > 1 gate. However, that fix was incomplete
because we don't want to use a decomposer if force_consolidate is set to
True. The 2q decomposer is used to compute the number of basis gates for
a 2q decomposition of the unitary based on the unitary's coordinates in
the weyl chamber. This basis gate count is then used as a heuristic in
the pass on whether to consolidate the block to a UnitaryGate or not.
Specifically, if the number of basis gates for the weyl coordinates is
less than the number of basis gates in the block then we consolidate
the block into a `UnitaryGate`. With `force_consolidate=True` all of this
work, including the initialization of the decomposers (which isn't
free), is wasted since we know we're going to consolidate.

This commit adjusts the logic in the pass so that the decomposer is
set to None when `force_consolidate=True` and the pass skips all usage
of the decomposer internally when force_consolidate is set to True.

Internally the rust code of the pass isn't handling the interface as
ideally as I would like. The rust interface I'd like is to define
the decomposer interface as something like:

```rust
enum DecomposerInput {
    ForceConsolidate,
    Decomposer(DecomposerType),
}
```

and removing the dedicated force_consolidate argument to make the use
of a decomposer and force consolidate mutally exclusive.
However, this isn't compatible with the pyo3 boundary layer. Since the
inner function of consolidate blocks is oddly the `py_` function to
avoid a larger refactor this opted to just make the decomposer arg an
`Option<DecomposerType>` and do a runtime check around the value and
`force_consolidate`. This has some additional overhead but in practice
it's negligible.

Author: mtreinish

crates/transpiler/src/passes/consolidate_blocks.rs

@@ -223,7 +223,7 @@ impl PhysQargsMap {
 #[pyo3(name = "consolidate_blocks", signature = (dag, decomposer, basis_gate_name, force_consolidate, target=None, basis_gates=None, blocks=None, runs=None, qubit_map=None))]
 fn py_run_consolidate_blocks(
     dag: &mut DAGCircuit,
-    decomposer: DecomposerType,
+    decomposer: Option<DecomposerType>,
     basis_gate_name: &str,
     force_consolidate: bool,
     target: Option<&Target>,
@@ -232,6 +232,11 @@ fn py_run_consolidate_blocks(
     runs: Option<Vec<Vec<usize>>>,
     qubit_map: Option<Vec<PhysicalQubit>>,
 ) -> PyResult<()> {
+    // If we don't have a decomposer and force consolidate is not set then there is not any
+    // consolidation to do.
+    if decomposer.is_none() && !force_consolidate {
+        return Ok(());
+    }
     // The node indices that enter from `blocks` and `runs` come from Python space, and we can't
     // trust that they come from a correct analysis (or the block/run collection might have been
     // invalidated). Rather than panicking, we should raise Python-space exceptions. We don't have
@@ -408,14 +413,23 @@ fn py_run_consolidate_blocks(
             ];
             let matrix = blocks_to_matrix(dag, &block, block_index_map).ok();
             if let Some(matrix) = matrix {
-                let num_basis_gates = match decomposer {
-                    DecomposerType::TwoQubitBasis(ref decomp) => decomp.num_basis_gates_inner(
-                        nalgebra_array_view::<Complex64, U4, U4>(matrix.as_view()),
-                    )?,
-                    DecomposerType::TwoQubitControlledU(ref decomp) => decomp
-                        .num_basis_gates_inner(nalgebra_array_view::<Complex64, U4, U4>(
-                            matrix.as_view(),
-                        ))?,
+                let num_basis_gates = if let Some(ref decomposer) = decomposer {
+                    match decomposer {
+                        DecomposerType::TwoQubitBasis(decomp) => decomp.num_basis_gates_inner(
+                            nalgebra_array_view::<Complex64, U4, U4>(matrix.as_view()),
+                        )?,
+                        DecomposerType::TwoQubitControlledU(decomp) => decomp
+                            .num_basis_gates_inner(nalgebra_array_view::<Complex64, U4, U4>(
+                                matrix.as_view(),
+                            ))?,
+                    }
+                } else {
+                    // If we don't have a decomposer set we have force_consolidate set.
+                    // This value doesn't matter since we're always going to consolidate,
+                    // but usize::MAX is selected as a safeguard against a logic bug since
+                    // with that value we'll only consolidate if force_consolidate is true
+                    // in the absence of a decomposer.
+                    usize::MAX
                 };
 
                 if force_consolidate
@@ -559,19 +573,34 @@ pub fn run_consolidate_blocks(
     target: Option<&Target>,
 ) -> PyResult<()> {
     let approximation_degree = approximation_degree.unwrap_or(1.0);
-    let (decomposer, basis_gate) = get_decomposer_and_basis_gate(target, approximation_degree);
-    py_run_consolidate_blocks(
-        dag,
-        decomposer,
-        basis_gate.name(),
-        force_consolidate,
-        target,
-        None,
-        None,
-        None,
-        // TODO: this doesn't handle the possibility of control-flow operations yet.
-        None,
-    )
+    if force_consolidate {
+        py_run_consolidate_blocks(
+            dag,
+            None,
+            "cx",
+            force_consolidate,
+            target,
+            None,
+            None,
+            None,
+            // TODO: this doesn't handle the possibility of control-flow operations yet.
+            None,
+        )
+    } else {
+        let (decomposer, basis_gate) = get_decomposer_and_basis_gate(target, approximation_degree);
+        py_run_consolidate_blocks(
+            dag,
+            Some(decomposer),
+            basis_gate.name(),
+            force_consolidate,
+            target,
+            None,
+            None,
+            None,
+            // TODO: this doesn't handle the possibility of control-flow operations yet.
+            None,
+        )
+    }
 }
 
 pub fn consolidate_blocks_mod(m: &Bound<PyModule>) -> PyResult<()> {

qiskit/transpiler/passes/optimization/consolidate_blocks.py

@@ -123,24 +123,23 @@ def __init__(
                     basis_fidelity=approximation_degree or 1.0,
                 )
                 self.basis_gate_name = next(iter(kak_gates))
-            elif force_consolidate:
-                # if we haven't found a decomposer but consolidation is forced,
-                # pick a default
-                self.decomposer = TwoQubitBasisDecomposer(CXGate())
-                self.basis_gate_name = "cx"
             else:
                 self.decomposer = None
-        else:
+                self.basis_gate_name = "cx"
+        elif not force_consolidate:
             self.decomposer = TwoQubitBasisDecomposer(CXGate())
             self.basis_gate_name = "cx"
+        else:
+            self.decomposer = None
+            self.basis_gate_name = "cx"
 
     def run(self, dag):
         """Run the ConsolidateBlocks pass on `dag`.
 
         Iterate over each block and replace it with an equivalent Unitary
         on the same wires.
         """
-        if self.decomposer is None:
+        if not self.force_consolidate and self.decomposer is None:
             return dag
 
         blocks = self.property_set["block_list"]
@@ -153,9 +152,13 @@ def run(self, dag):
         qubit_map = self.property_set.get(self._QUBIT_MAP_KEY, None)
         if qubit_map is None:
             qubit_map = list(range(dag.num_qubits()))
+        if self.decomposer is not None:
+            decomposer = self.decomposer._inner_decomposer
+        else:
+            decomposer = None
         consolidate_blocks(
             dag,
-            self.decomposer._inner_decomposer,
+            decomposer,
             self.basis_gate_name,
             self.force_consolidate,
             target=self.target,