Skip to content

Commit

Permalink
Fix: Make py_op private.
Browse files Browse the repository at this point in the history
- Add views for `py_op`.
- Remove stale saching being done in `apply_operation_*` in `DAGCircuit`.
- Adapt the code to new changes.
- Add missing docstring
  • Loading branch information
raynelfss committed Dec 10, 2024
1 parent b337cf3 commit 473ca03
Show file tree
Hide file tree
Showing 11 changed files with 32 additions and 108 deletions.
31 changes: 8 additions & 23 deletions crates/accelerate/src/barrier_before_final_measurement.rs
Original file line number Diff line number Diff line change
Expand Up @@ -84,29 +84,14 @@ pub fn barrier_before_final_measurements(
instruction: new_barrier.unbind(),
};
let qargs: Vec<Qubit> = (0..dag.num_qubits() as u32).map(Qubit).collect();
#[cfg(feature = "cache_pygates")]
{
dag.apply_operation_back(
py,
PackedOperation::from_instruction(Box::new(new_barrier_py_inst)),
qargs.as_slice(),
&[],
None,
ExtraInstructionAttributes::new(label, None, None, None),
Some(new_barrier.unbind()),
)?;
}
#[cfg(not(feature = "cache_pygates"))]
{
dag.apply_operation_back(
py,
PackedOperation::from_instruction(Box::new(new_barrier_py_inst)),
qargs.as_slice(),
&[],
None,
ExtraInstructionAttributes::new(label, None, None, None),
)?;
}
dag.apply_operation_back(
py,
PackedOperation::from_instruction(Box::new(new_barrier_py_inst)),
qargs.as_slice(),
&[],
None,
ExtraInstructionAttributes::new(label, None, None, None),
)?;
for inst in final_packed_ops {
dag.push_back(py, inst)?;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -74,8 +74,6 @@ pub(super) fn compose_transforms<'a>(
Some(gate_obj.params)
},
gate_obj.extra_attrs,
#[cfg(feature = "cache_pygates")]
Some(gate.into()),
)?;
mapped_instructions.insert((gate_name, gate_num_qubits), (placeholder_params, dag));

Expand Down
12 changes: 0 additions & 12 deletions crates/accelerate/src/basis/basis_translator/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -481,8 +481,6 @@ fn apply_translation(
node_carg,
(!new_op.params.is_empty()).then_some(new_op.params),
new_op.extra_attrs,
#[cfg(feature = "cache_pygates")]
None,
)?;
} else {
out_dag.apply_operation_back(
Expand All @@ -498,8 +496,6 @@ fn apply_translation(
.collect(),
),
node_obj.extra_attrs().clone(),
#[cfg(feature = "cache_pygates")]
None,
)?;
}
continue;
Expand All @@ -523,8 +519,6 @@ fn apply_translation(
.collect(),
),
node_obj.extra_attrs().clone(),
#[cfg(feature = "cache_pygates")]
None,
)?;
continue;
}
Expand All @@ -543,8 +537,6 @@ fn apply_translation(
.collect(),
),
node_obj.extra_attrs().clone(),
#[cfg(feature = "cache_pygates")]
None,
)?;
continue;
}
Expand Down Expand Up @@ -642,8 +634,6 @@ fn replace_node(
&new_clbits,
(!new_params.is_empty()).then_some(new_params),
new_extra_props,
#[cfg(feature = "cache_pygates")]
None,
)?;
}
dag.add_global_phase(py, target_dag.global_phase())?;
Expand Down Expand Up @@ -742,8 +732,6 @@ fn replace_node(
&new_clbits,
(!new_params.is_empty()).then_some(new_params),
inner_node.extra_attrs().clone(),
#[cfg(feature = "cache_pygates")]
None,
)?;
}

Expand Down
2 changes: 0 additions & 2 deletions crates/accelerate/src/elide_permutations.rs
Original file line number Diff line number Diff line change
Expand Up @@ -89,8 +89,6 @@ fn run(py: Python, dag: &mut DAGCircuit) -> PyResult<Option<(DAGCircuit, Vec<usi
cargs,
(!inst.params_view().is_empty()).then_some(inst.params_view().into()),
inst.extra_attrs().clone(),
#[cfg(feature = "cache_pygates")]
None,
)?;
}
}
Expand Down
4 changes: 1 addition & 3 deletions crates/accelerate/src/gate_direction.rs
Original file line number Diff line number Diff line change
Expand Up @@ -406,7 +406,7 @@ fn has_calibration_for_op_node(
.collect(),
extra_attrs: packed_inst.extra_attrs().clone(),
#[cfg(feature = "cache_pygates")]
py_op: packed_inst.py_op.clone(),
py_op: packed_inst.py_op().clone(),
},
sort_key: "".into_py(py),
},
Expand Down Expand Up @@ -479,8 +479,6 @@ fn apply_operation_back(
&[],
param,
ExtraInstructionAttributes::default(),
#[cfg(feature = "cache_pygates")]
None,
)?;

Ok(())
Expand Down
2 changes: 0 additions & 2 deletions crates/accelerate/src/twirling.rs
Original file line number Diff line number Diff line change
Expand Up @@ -353,8 +353,6 @@ fn generate_twirled_circuit(
),
inst.extra_attrs().clone(),
);
#[cfg(feature = "cache_pygates")]
new_inst.py_op.set(new_inst_obj).unwrap();
out_circ.push(py, new_inst)?;
} else {
out_circ.push(py, inst.clone())?;
Expand Down
4 changes: 0 additions & 4 deletions crates/accelerate/src/unitary_synthesis.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,6 @@
#![allow(clippy::too_many_arguments)]

use std::f64::consts::PI;
#[cfg(feature = "cache_pygates")]
use std::sync::OnceLock;

use approx::relative_eq;
use hashbrown::{HashMap, HashSet};
Expand Down Expand Up @@ -320,8 +318,6 @@ fn py_run_main_loop(
&[],
Some(new_params),
ExtraInstructionAttributes::default(),
#[cfg(feature = "cache_pygates")]
None,
)?;
}
out_dag.add_global_phase(py, &Param::Float(sequence.global_phase))?;
Expand Down
13 changes: 3 additions & 10 deletions crates/circuit/src/circuit_data.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,6 @@
// copyright notice, and modified files need to carry a notice indicating
// that they have been altered from the originals.

#[cfg(feature = "cache_pygates")]
use std::sync::OnceLock;

use crate::bit_data::BitData;
use crate::circuit_instruction::{
CircuitInstruction, ExtraInstructionAttributes, OperationFromPython,
Expand Down Expand Up @@ -405,7 +402,7 @@ impl CircuitData {
*inst.extra_attrs_mut() = result.extra_attrs;
#[cfg(feature = "cache_pygates")]
{
inst.py_op = py_op.unbind().into();
*inst.py_op_mut() = py_op.unbind().into();
}
}
Ok(())
Expand Down Expand Up @@ -515,7 +512,7 @@ impl CircuitData {
.collect(),
extra_attrs: inst.extra_attrs().clone(),
#[cfg(feature = "cache_pygates")]
py_op: inst.py_op.clone(),
py_op: inst.py_op().clone(),
}
.into_py(py)
};
Expand Down Expand Up @@ -1374,7 +1371,7 @@ impl CircuitData {
// Standard gates can all rebuild their definitions, so if the
// cached py_op exists, update the `params` attribute and clear out
// any existing cache.
if let Some(borrowed) = previous.py_op.get() {
if let Some(borrowed) = previous.py_op().get() {
borrowed
.bind(py)
.getattr(params_attr)?
Expand Down Expand Up @@ -1447,10 +1444,6 @@ impl CircuitData {
.params_view_mut()
.swap_with_slice(&mut new_op.params);
*previous.extra_attrs_mut() = new_op.extra_attrs;
#[cfg(feature = "cache_pygates")]
{
previous.py_op = op.into_py(py).into();
}
for uuid in uuids.iter() {
self.param_table.add_use(*uuid, usage)?
}
Expand Down
3 changes: 0 additions & 3 deletions crates/circuit/src/converters.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,6 @@
// copyright notice, and modified files need to carry a notice indicating
// that they have been altered from the originals.

#[cfg(feature = "cache_pygates")]
use std::sync::OnceLock;

use ::pyo3::prelude::*;
use hashbrown::HashMap;
use pyo3::{
Expand Down
51 changes: 5 additions & 46 deletions crates/circuit/src/dag_circuit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3253,7 +3253,7 @@ def _format(operand):
.into();
#[cfg(feature = "cache_pygates")]
{
new_inst.py_op = new_op.unbind().into();
*new_inst.py_op_mut() = new_op.unbind().into();
}
}
}
Expand All @@ -3271,10 +3271,6 @@ def _format(operand):
new_inst
.extra_attrs_mut()
.set_condition(new_condition.clone());
#[cfg(feature = "cache_pygates")]
{
new_inst.py_op.take();
}
match new_inst.op().view() {
OperationRef::Instruction(py_inst) => {
py_inst
Expand Down Expand Up @@ -3351,7 +3347,7 @@ def _format(operand):
node.instruction.extra_attrs = new_weight.extra_attrs().clone();
#[cfg(feature = "cache_pygates")]
{
node.instruction.py_op = new_weight.py_op.clone();
node.instruction.py_op = new_weight.py_op().clone();
}
Ok(node.into_py(py))
} else {
Expand Down Expand Up @@ -5065,19 +5061,8 @@ impl DAGCircuit {
cargs: &[Clbit],
params: Option<SmallVec<[Param; 3]>>,
extra_attrs: ExtraInstructionAttributes,
#[cfg(feature = "cache_pygates")] py_op: Option<PyObject>,
) -> PyResult<NodeIndex> {
self.inner_apply_op(
py,
op,
qargs,
cargs,
params,
extra_attrs,
#[cfg(feature = "cache_pygates")]
py_op,
false,
)
self.inner_apply_op(py, op, qargs, cargs, params, extra_attrs, false)
}

/// Apply a [PackedOperation] to the front of the circuit.
Expand All @@ -5089,19 +5074,8 @@ impl DAGCircuit {
cargs: &[Clbit],
params: Option<SmallVec<[Param; 3]>>,
extra_attrs: ExtraInstructionAttributes,
#[cfg(feature = "cache_pygates")] py_op: Option<PyObject>,
) -> PyResult<NodeIndex> {
self.inner_apply_op(
py,
op,
qargs,
cargs,
params,
extra_attrs,
#[cfg(feature = "cache_pygates")]
py_op,
true,
)
self.inner_apply_op(py, op, qargs, cargs, params, extra_attrs, true)
}

#[inline]
Expand All @@ -5114,7 +5088,6 @@ impl DAGCircuit {
cargs: &[Clbit],
params: Option<SmallVec<[Param; 3]>>,
extra_attrs: ExtraInstructionAttributes,
#[cfg(feature = "cache_pygates")] py_op: Option<PyObject>,
front: bool,
) -> PyResult<NodeIndex> {
// Check that all qargs are within an acceptable range
Expand All @@ -5140,13 +5113,6 @@ impl DAGCircuit {
}
Ok(())
})?;

#[cfg(feature = "cache_pygates")]
let py_op = if let Some(py_op) = py_op {
py_op.into()
} else {
OnceLock::new()
};
let packed_instruction = PackedInstruction::new(
op,
self.qargs_interner.insert(qargs),
Expand Down Expand Up @@ -5642,7 +5608,7 @@ impl DAGCircuit {
.collect(),
extra_attrs: packed.extra_attrs().clone(),
#[cfg(feature = "cache_pygates")]
py_op: packed.py_op.clone(),
py_op: packed.py_op().clone(),
},
sort_key: format!("{:?}", self.sort_key(id)).into_py(py),
},
Expand Down Expand Up @@ -6923,9 +6889,6 @@ impl DAGCircuit {
)));
}

#[cfg(feature = "cache_pygates")]
let mut py_op_cache = Some(op.clone().unbind());

let mut extra_attrs = new_op.extra_attrs.clone();
// If either operation is a control-flow operation, propagate_condition is ignored
if propagate_condition
Expand Down Expand Up @@ -6963,10 +6926,6 @@ impl DAGCircuit {
old_condition.downcast_bound::<PyTuple>(py)?,
)?;
}
#[cfg(feature = "cache_pygates")]
{
py_op_cache = None;
}
}
};
if new_wires != current_wires {
Expand Down
16 changes: 15 additions & 1 deletion crates/circuit/src/packed_instruction.rs
Original file line number Diff line number Diff line change
Expand Up @@ -514,7 +514,7 @@ pub struct PackedInstruction {
/// requires the GIL to even `get` (of course!), which makes implementing `Clone` hard for us.
/// We can revisit once we're on PyO3 0.22+ and have been able to disable its `py-clone`
/// feature.
pub py_op: OnceLock<Py<PyAny>>,
py_op: OnceLock<Py<PyAny>>,
}

impl PackedInstruction {
Expand Down Expand Up @@ -554,10 +554,17 @@ impl PackedInstruction {
self.clbits
}

/// Retrieves an immutable reference to the extra_attributes of this instruction.
pub fn extra_attrs(&self) -> &ExtraInstructionAttributes {
&self.extra_attrs
}

/// Retrieves the cached py_gate immutably.
#[cfg(feature = "cache_pygates")]
pub fn py_op(&self) -> &OnceLock<PyObject> {
&self.py_op
}

// Setters

/// Retrieves an immutable reference to the instruction's underlying operation.
Expand All @@ -579,6 +586,7 @@ impl PackedInstruction {
&mut self.clbits
}

/// Retrieves a mutable reference to the extra_attributes of this instruction.
pub fn extra_attrs_mut(&mut self) -> &mut ExtraInstructionAttributes {
#[cfg(feature = "cache_pygates")]
{
Expand All @@ -587,6 +595,12 @@ impl PackedInstruction {
&mut self.extra_attrs
}

/// Retrieves the cached py_gate immutably.
#[cfg(feature = "cache_pygates")]
pub fn py_op_mut(&mut self) -> &mut OnceLock<PyObject> {
&mut self.py_op
}

/// Access the standard gate in this `PackedInstruction`, if it is one. If the instruction
/// refers to a Python-space object, `None` is returned.
#[inline]
Expand Down

0 comments on commit 473ca03

Please sign in to comment.