From cf7d8b09681c400c2213c6c35722f85a7f7c63b4 Mon Sep 17 00:00:00 2001 From: Julien Gacon Date: Mon, 28 Oct 2024 13:23:00 +0100 Subject: [PATCH] implement review comments --- crates/accelerate/src/circuit_library/mod.rs | 1 - .../src/circuit_library/pauli_evolution.rs | 95 ++++++++++++------- .../accelerate/src/circuit_library/utils.rs | 58 ----------- qiskit/synthesis/evolution/product_formula.py | 8 +- .../pauli-evo-plugins-612850146c3f7d49.yaml | 2 +- 5 files changed, 65 insertions(+), 99 deletions(-) delete mode 100644 crates/accelerate/src/circuit_library/utils.rs diff --git a/crates/accelerate/src/circuit_library/mod.rs b/crates/accelerate/src/circuit_library/mod.rs index b943881e4a14..3897167f9489 100644 --- a/crates/accelerate/src/circuit_library/mod.rs +++ b/crates/accelerate/src/circuit_library/mod.rs @@ -16,7 +16,6 @@ mod entanglement; mod pauli_evolution; mod pauli_feature_map; mod quantum_volume; -mod utils; pub fn circuit_library(m: &Bound) -> PyResult<()> { m.add_wrapped(wrap_pyfunction!(pauli_evolution::py_pauli_evolution))?; diff --git a/crates/accelerate/src/circuit_library/pauli_evolution.rs b/crates/accelerate/src/circuit_library/pauli_evolution.rs index 243b70e6edee..9b0a43cc04b1 100644 --- a/crates/accelerate/src/circuit_library/pauli_evolution.rs +++ b/crates/accelerate/src/circuit_library/pauli_evolution.rs @@ -13,13 +13,11 @@ use pyo3::prelude::*; use pyo3::types::{PyList, PyString, PyTuple}; use qiskit_circuit::circuit_data::CircuitData; -use qiskit_circuit::operations::{multiply_param, radd_param, Param, StandardGate}; +use qiskit_circuit::operations::{multiply_param, radd_param, Param, PyInstruction, StandardGate}; use qiskit_circuit::packed_instruction::PackedOperation; -use qiskit_circuit::{Clbit, Qubit}; +use qiskit_circuit::{imports, Clbit, Qubit}; use smallvec::{smallvec, SmallVec}; -use crate::circuit_library::utils; - // custom types for a more readable code type StandardInstruction = (StandardGate, SmallVec<[Param; 3]>, SmallVec<[Qubit; 2]>); type Instruction = ( @@ -115,7 +113,7 @@ fn two_qubit_evolution<'a>( "zx" => Box::new(std::iter::once((StandardGate::RZXGate, param, qubits))), "yy" => Box::new(std::iter::once((StandardGate::RYYGate, param, qubits))), "zz" => Box::new(std::iter::once((StandardGate::RZZGate, param, qubits))), - // note that the CX modes (do_fountain=true/false) give the same circuit for a 2-qubit + // Note: the CX modes (do_fountain=true/false) give the same circuit for a 2-qubit // Pauli, so we just set it to false here _ => Box::new(multi_qubit_evolution(pauli, indices, time, false, false)), } @@ -135,22 +133,25 @@ fn multi_qubit_evolution( .collect(); // get the basis change: x -> HGate, y -> SXdgGate, z -> nothing - let basis_change = active_paulis - .clone() - .into_iter() + let basis_change: Vec = active_paulis + .iter() .filter(|(p, _)| *p != 'z') .map(|(p, q)| match p { - 'x' => (StandardGate::HGate, smallvec![], smallvec![q]), - 'y' => (StandardGate::SXdgGate, smallvec![], smallvec![q]), + 'x' => (StandardGate::HGate, smallvec![], smallvec![q.clone()]), + 'y' => (StandardGate::SXdgGate, smallvec![], smallvec![q.clone()]), _ => unreachable!("Invalid Pauli string."), // "z" and "i" have been filtered out - }); + }) + .collect(); // get the inverse basis change - let inverse_basis_change = basis_change.clone().map(|(gate, _, qubit)| match gate { - StandardGate::HGate => (gate, smallvec![], qubit), - StandardGate::SXdgGate => (StandardGate::SXGate, smallvec![], qubit), - _ => unreachable!("Invalid basis-changing Clifford."), - }); + let inverse_basis_change: Vec = basis_change + .iter() + .map(|(gate, _, qubit)| match gate { + StandardGate::HGate => (StandardGate::HGate, smallvec![], qubit.clone()), + StandardGate::SXdgGate => (StandardGate::SXGate, smallvec![], qubit.clone()), + _ => unreachable!("Invalid basis-changing Clifford."), + }) + .collect(); // get the CX propagation up to the first qubit, and down let (chain_up, chain_down) = match do_fountain { @@ -178,6 +179,7 @@ fn multi_qubit_evolution( // and finally chain everything together basis_change + .into_iter() .chain(chain_down) .chain(z_rotation) .chain(chain_up) @@ -215,19 +217,20 @@ fn multi_qubit_evolution( /// Returns: /// Circuit data for to implement the evolution. #[pyfunction] -#[pyo3(signature = (num_qubits, sparse_paulis, insert_barriers=false, do_fountain=false))] +#[pyo3(name = "pauli_evolution", signature = (num_qubits, sparse_paulis, insert_barriers=false, do_fountain=false))] pub fn py_pauli_evolution( - py: Python, num_qubits: i64, sparse_paulis: &Bound, insert_barriers: bool, do_fountain: bool, ) -> PyResult { + let py = sparse_paulis.py(); let num_paulis = sparse_paulis.len(); let mut paulis: Vec = Vec::with_capacity(num_paulis); let mut indices: Vec> = Vec::with_capacity(num_paulis); let mut times: Vec = Vec::with_capacity(num_paulis); let mut global_phase = Param::Float(0.0); + let mut modified_phase = false; // keep track of whether we modified the phase for el in sparse_paulis.iter() { let tuple = el.downcast::()?; @@ -235,22 +238,18 @@ pub fn py_pauli_evolution( let time = Param::extract_no_coerce(&tuple.get_item(2)?)?; if pauli.as_str().chars().all(|p| p == 'i') { - global_phase = radd_param(global_phase, time, py); // apply factor -1 at the end + global_phase = radd_param(global_phase, time, py); + modified_phase = true; continue; } paulis.push(pauli); times.push(time); // note we do not multiply by 2 here, this is done Python side! - indices.push( - tuple - .get_item(1)? - .downcast::()? - .iter() - .map(|index| index.extract::()) - .collect::>()?, - ); + indices.push(tuple.get_item(1)?.extract::>()?) } + let barrier = get_barrier(py, num_qubits as u32); + let evos = paulis.iter().enumerate().zip(indices).zip(times).flat_map( |(((i, pauli), qubits), time)| { let as_packed = pauli_evolution(pauli, qubits, time, false, do_fountain).map( @@ -263,16 +262,23 @@ pub fn py_pauli_evolution( )) }, ); - as_packed.chain(utils::maybe_barrier( - py, - num_qubits as u32, - insert_barriers && i < (num_paulis - 1), // do not add barrier on final block - )) + + // this creates an iterator containing a barrier only if required, otherwise it is empty + let maybe_barrier = (insert_barriers && i < (num_paulis - 1)) + .then_some(Ok(barrier.clone())) + .into_iter(); + as_packed.chain(maybe_barrier) }, ); - // apply factor -1 for global phase - global_phase = multiply_param(&global_phase, -1.0, py); + // When handling all-identity Paulis above, we added the time as global phase. + // However, the all-identity Paulis should add a negative phase, as they implement + // exp(-i t I). We apply the negative sign here, to only do a single (-1) multiplication, + // instead of doing it every time we find an all-identity Pauli. + if modified_phase { + global_phase = multiply_param(&global_phase, -1.0, py); + } + CircuitData::from_packed_operations(py, num_qubits as u32, 0, evos, global_phase) } @@ -301,3 +307,24 @@ fn cx_fountain( ) })) } + +fn get_barrier(py: Python, num_qubits: u32) -> Instruction { + let barrier_cls = imports::BARRIER.get_bound(py); + let barrier = barrier_cls + .call1((num_qubits,)) + .expect("Could not create Barrier Python-side"); + let barrier_inst = PyInstruction { + qubits: num_qubits, + clbits: 0, + params: 0, + op_name: "barrier".to_string(), + control_flow: false, + instruction: barrier.into(), + }; + ( + barrier_inst.into(), + smallvec![], + (0..num_qubits).map(Qubit).collect(), + vec![], + ) +} diff --git a/crates/accelerate/src/circuit_library/utils.rs b/crates/accelerate/src/circuit_library/utils.rs deleted file mode 100644 index 4df8d64865f0..000000000000 --- a/crates/accelerate/src/circuit_library/utils.rs +++ /dev/null @@ -1,58 +0,0 @@ -// This code is part of Qiskit. -// -// (C) Copyright IBM 2024 -// -// This code is licensed under the Apache License, Version 2.0. You may -// obtain a copy of this license in the LICENSE.txt file in the root directory -// of this source tree or at http://www.apache.org/licenses/LICENSE-2.0. -// -// Any modifications or derivative works of this code must retain this -// copyright notice, and modified files need to carry a notice indicating -// that they have been altered from the originals. - -use pyo3::prelude::*; -use qiskit_circuit::{ - imports, - operations::{Param, PyInstruction}, - packed_instruction::PackedOperation, - Clbit, Qubit, -}; -use smallvec::{smallvec, SmallVec}; - -type Instruction = ( - PackedOperation, - SmallVec<[Param; 3]>, - Vec, - Vec, -); - -/// Get an iterator that returns a barrier or an empty element. -pub fn maybe_barrier( - py: Python, - num_qubits: u32, - insert_barriers: bool, -) -> Box>> { - // TODO could speed this up by only defining the barrier class once - if !insert_barriers { - Box::new(std::iter::empty()) - } else { - let barrier_cls = imports::BARRIER.get_bound(py); - let barrier = barrier_cls - .call1((num_qubits,)) - .expect("Could not create Barrier Python-side"); - let barrier_inst = PyInstruction { - qubits: num_qubits, - clbits: 0, - params: 0, - op_name: "barrier".to_string(), - control_flow: false, - instruction: barrier.into(), - }; - Box::new(std::iter::once(Ok(( - barrier_inst.into(), - smallvec![], - (0..num_qubits).map(Qubit).collect(), - vec![] as Vec, - )))) - } -} diff --git a/qiskit/synthesis/evolution/product_formula.py b/qiskit/synthesis/evolution/product_formula.py index 2ac4f151feda..4de838215ff1 100644 --- a/qiskit/synthesis/evolution/product_formula.py +++ b/qiskit/synthesis/evolution/product_formula.py @@ -22,7 +22,7 @@ from qiskit.circuit.quantumcircuit import QuantumCircuit, ParameterValueType from qiskit.quantum_info import SparsePauliOp, Pauli from qiskit.utils.deprecation import deprecate_arg -from qiskit._accelerate.circuit_library import py_pauli_evolution +from qiskit._accelerate.circuit_library import pauli_evolution from .evolution_synthesis import EvolutionSynthesis @@ -151,9 +151,7 @@ def synthesize(self, evolution: PauliEvolutionGate) -> QuantumCircuit: else: # this is the fast path, where the whole evolution is constructed Rust-side cx_fountain = self._cx_structure == "fountain" - data = py_pauli_evolution( - num_qubits, pauli_rotations, self.insert_barriers, cx_fountain - ) + data = pauli_evolution(num_qubits, pauli_rotations, self.insert_barriers, cx_fountain) circuit = QuantumCircuit._from_circuit_data(data, add_regs=True) return circuit @@ -210,7 +208,7 @@ def _custom_evolution(self, num_qubits, pauli_rotations): local_pauli = (pauli_string, list(range(len(qubits))), coeff) # build the circuit Rust-side - data = py_pauli_evolution( + data = pauli_evolution( len(qubits), [local_pauli], False, diff --git a/releasenotes/notes/pauli-evo-plugins-612850146c3f7d49.yaml b/releasenotes/notes/pauli-evo-plugins-612850146c3f7d49.yaml index 4a01f7c75888..67ccdfbae3b5 100644 --- a/releasenotes/notes/pauli-evo-plugins-612850146c3f7d49.yaml +++ b/releasenotes/notes/pauli-evo-plugins-612850146c3f7d49.yaml @@ -2,7 +2,7 @@ features_quantum_info: - | Added :meth:`.SparsePauliOperator.to_sparse_list` to convert an operator into - a sparse list format. This works analogous to :meth:`.SparsePauliOperator.from_sparse_list`. + a sparse list format. This works inversely to :meth:`.SparsePauliOperator.from_sparse_list`. For example:: from qiskit.quantum_info import SparsePauliOp