Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix registering global_phase parameters when creating CircuitData #13538

Merged
merged 2 commits into from
Dec 9, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion crates/accelerate/src/twirling.rs
Original file line number Diff line number Diff line change
Expand Up @@ -267,7 +267,7 @@ fn generate_twirled_circuit(
custom_gate_map: Option<&CustomGateTwirlingMap>,
optimizer_target: Option<&Target>,
) -> PyResult<CircuitData> {
let mut out_circ = CircuitData::clone_empty_like(circ, None);
let mut out_circ = CircuitData::clone_empty_like(py, circ, None)?;

for inst in circ.data() {
if let Some(custom_gate_map) = custom_gate_map {
Expand Down
25 changes: 19 additions & 6 deletions crates/circuit/src/circuit_data.rs
Original file line number Diff line number Diff line change
Expand Up @@ -930,6 +930,7 @@ impl CircuitData {
instruction_iter.size_hint().0,
global_phase,
)?;

for item in instruction_iter {
let (operation, params, qargs, cargs) = item?;
let qubits = res.qargs_interner.insert_owned(qargs);
Expand Down Expand Up @@ -996,9 +997,13 @@ impl CircuitData {
qubits,
clbits,
param_table: ParameterTable::new(),
global_phase,
global_phase: Param::Float(0.0),
};

// use the global phase setter to ensure parameters are registered
// in the parameter table
res.set_global_phase(py, global_phase)?;

for inst in instruction_iter {
res.data.push(inst?);
res.track_instruction_parameters(py, res.data.len() - 1)?;
Expand Down Expand Up @@ -1040,6 +1045,7 @@ impl CircuitData {
instruction_iter.size_hint().0,
global_phase,
)?;

let no_clbit_index = res.cargs_interner.get_default();
for (operation, params, qargs) in instruction_iter {
let qubits = res.qargs_interner.insert(&qargs);
Expand Down Expand Up @@ -1073,8 +1079,13 @@ impl CircuitData {
qubits: BitData::new(py, "qubits".to_string()),
clbits: BitData::new(py, "clbits".to_string()),
param_table: ParameterTable::new(),
global_phase,
global_phase: Param::Float(0.0),
};

// use the global phase setter to ensure parameters are registered
// in the parameter table
res.set_global_phase(py, global_phase)?;

if num_qubits > 0 {
let qubit_cls = QUBIT.get_bound(py);
for _i in 0..num_qubits {
Expand Down Expand Up @@ -1528,16 +1539,18 @@ impl CircuitData {
/// * capacity - The capacity for instructions to use in the output `CircuitData`
/// If `None` the length of `other` will be used, if `Some` the integer
/// value will be used as the capacity.
pub fn clone_empty_like(other: &Self, capacity: Option<usize>) -> Self {
CircuitData {
pub fn clone_empty_like(py: Python, other: &Self, capacity: Option<usize>) -> PyResult<Self> {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change is necessary since CircuitData::set_global_phase requires the Python token. This function is so far only ever used in pauli_twirl_2q_gates, so it is a very non-intrusive change.

let mut empty = CircuitData {
data: Vec::with_capacity(capacity.unwrap_or(other.data.len())),
qargs_interner: other.qargs_interner.clone(),
cargs_interner: other.cargs_interner.clone(),
qubits: other.qubits.clone(),
clbits: other.clbits.clone(),
param_table: ParameterTable::new(),
global_phase: other.global_phase.clone(),
}
global_phase: Param::Float(0.0),
};
empty.set_global_phase(py, other.global_phase.clone())?;
Ok(empty)
}

/// Append a PackedInstruction to the circuit data.
Expand Down
14 changes: 14 additions & 0 deletions releasenotes/notes/fix-global-phase-assign-d05f182ed9ddcf57.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
---
fixes:
- |
Fixed a series of bugs when processing circuit with parameterized global phases,
where upon assignment the global phase was not correctly assigned.
Known cases this affected include:
* assigning parameters after calling :meth:`.QuantumCircuit.decompose` on a circuit,
where the decomposition introduces a global phase
* assigning parameters on a circuit constructed from a DAG via :func:`.dag_to_circuit`
* assigning parameters on circuits created with :func:`.pauli_twirl_2q_gates`, where
the circuit to be twirled had a parameterized global phase
Fixed `#13534 <https://github.com/Qiskit/qiskit/issues/13534>`__.
28 changes: 28 additions & 0 deletions test/python/circuit/test_parameters.py
Original file line number Diff line number Diff line change
Expand Up @@ -1246,6 +1246,18 @@ def test_repeated_gates_to_dag_and_back(self):
bound_test_qc = test_qc.assign_parameters({theta: 1})
self.assertEqual(len(bound_test_qc.parameters), 0)

def test_global_phase_to_dag_and_back(self):
"""Test global phase parameters are correctly handled to dag and back."""
from qiskit.converters import circuit_to_dag, dag_to_circuit

theta = Parameter("theta")
qc = QuantumCircuit(global_phase=theta)

test_qc = dag_to_circuit(circuit_to_dag(qc))

bound_test_qc = test_qc.assign_parameters({theta: 1})
self.assertEqual(bound_test_qc.global_phase, 1)

def test_rebinding_instruction_copy(self):
"""Test rebinding a copied instruction does not modify the original."""

Expand Down Expand Up @@ -1617,6 +1629,22 @@ def test_sub_allow_unknown_parameters(self):
subbed = x.subs({y: z}, allow_unknown_parameters=True)
self.assertEqual(subbed, x)

def test_decompose_with_global_phase(self):
"""Test decomposing a circuit which introduces a global phase is correctly bound.
Regression test of #13534.
"""
x = Parameter("x")
qc = QuantumCircuit(1)
qc.rz(x, 0)

bound = qc.decompose().assign_parameters([1])

expect = QuantumCircuit(1)
expect.rz(1, 0)

self.assertEqual(expect.decompose(), bound)


def _construct_circuit(param, qr):
qc = QuantumCircuit(qr)
Expand Down
12 changes: 11 additions & 1 deletion test/python/circuit/test_twirling.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
import ddt
import numpy as np

from qiskit.circuit import QuantumCircuit, pauli_twirl_2q_gates, Gate
from qiskit.circuit import QuantumCircuit, pauli_twirl_2q_gates, Gate, Parameter
from qiskit.circuit.library import (
CXGate,
ECRGate,
Expand Down Expand Up @@ -210,3 +210,13 @@ def test_error_on_parameterized_gate(self):
qc = QuantumCircuit(5)
with self.assertRaises(QiskitError):
pauli_twirl_2q_gates(qc, [RZXGate(3.24)])

def test_with_global_phase(self):
"""Test twirling a circuit with parameterized global phase."""

x = Parameter("x")
qc = QuantumCircuit(2, global_phase=x)
res = pauli_twirl_2q_gates(qc, seed=2)
bound = res.assign_parameters([1])

self.assertEqual(bound.global_phase, 1)
Loading