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

Port circuit_to_dag to Rust #13036

Merged
merged 28 commits into from
Sep 10, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
7a9553c
Initial: Add add_from_iter method to DAGCircuit
raynelfss Aug 25, 2024
401abc3
Fix: leverage new methods in layers
raynelfss Aug 25, 2024
15eaa1c
Fix: Keep track of Vars for add_from_iter
raynelfss Aug 25, 2024
6ba431e
Fix: Incorrect modification of last nodes in `add_from_iter`.
raynelfss Aug 26, 2024
9464950
Fix: Cycling edges in when adding vars.
raynelfss Aug 26, 2024
5d7c5ab
Fix: Remove set collecting all nodes to be connected.
raynelfss Aug 26, 2024
f5d120b
Fix: Adapt to #13033
raynelfss Aug 27, 2024
1fd902e
Refactor: `add_from_iter` is now called `extend` to stick with `Rust`…
raynelfss Sep 4, 2024
144955c
Fix: Remove duplicate vars check
raynelfss Sep 4, 2024
91c674b
Fix: Corrections from code review.
raynelfss Sep 6, 2024
4270f0a
Fix: Improper use of `Entry API`.
raynelfss Sep 6, 2024
0481bfc
Initial: Add add_from_iter method to DAGCircuit
raynelfss Aug 25, 2024
5bc6faa
Initial: Expose `CircuitData` interners and registers to the `qiskit-…
raynelfss Aug 20, 2024
d4fde24
FIx: Expose immutable views of interners, registers and global phase.
raynelfss Aug 21, 2024
e4b5d4c
Refactor: Use naming convention for getters.
raynelfss Aug 21, 2024
c5473cd
Docs: Apply suggestions from code review
raynelfss Aug 22, 2024
b8b72a0
Initial: Add `circuit_to_dag` in rust.
raynelfss Aug 26, 2024
d6661c8
Fix: Move qubit ordering check to python
raynelfss Aug 26, 2024
300d7a4
Fix: Regenerate `BitData` instances dynamically instead of cloning.
raynelfss Aug 26, 2024
f9a6fe3
Fix: Make use of `copy_operations`
raynelfss Aug 27, 2024
b6262d5
FIx: Adapt to 13033
raynelfss Aug 28, 2024
20bc83a
Fix: Correctly map qubits and clbits to the `DAGCircuit`.
raynelfss Aug 29, 2024
df0987f
Add: `DAGCircuit` from `CircuitData`
raynelfss Sep 3, 2024
68e15b5
Fix: Make `QuantumCircuitData` public.
raynelfss Sep 4, 2024
e6d582d
Fix: Use `intern!` when extracting attributes from Python.
raynelfss Sep 4, 2024
4f520b3
Fix: Copy instructions in place instead of copying circuit data.
raynelfss Sep 6, 2024
fa34c5a
Fix: Remove second re-mapping of bits
raynelfss Sep 10, 2024
29adcfe
Update crates/circuit/src/dag_circuit.rs
raynelfss Sep 10, 2024
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
91 changes: 91 additions & 0 deletions crates/circuit/src/converters.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,91 @@
// This code is part of Qiskit.
//
// (C) Copyright IBM 2023, 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 hashbrown::HashMap;
use pyo3::{
intern,
types::{PyDict, PyList},
};

use crate::{circuit_data::CircuitData, dag_circuit::DAGCircuit};

/// An extractable representation of a QuantumCircuit reserved only for
/// conversion purposes.
#[derive(Debug, Clone)]
pub struct QuantumCircuitData<'py> {
pub data: CircuitData,
pub name: Option<Bound<'py, PyAny>>,
pub calibrations: Option<HashMap<String, Py<PyDict>>>,
pub metadata: Option<Bound<'py, PyAny>>,
pub qregs: Option<Bound<'py, PyList>>,
pub cregs: Option<Bound<'py, PyList>>,
pub input_vars: Vec<Bound<'py, PyAny>>,
pub captured_vars: Vec<Bound<'py, PyAny>>,
pub declared_vars: Vec<Bound<'py, PyAny>>,
}

impl<'py> FromPyObject<'py> for QuantumCircuitData<'py> {
fn extract_bound(ob: &Bound<'py, PyAny>) -> PyResult<Self> {
let py = ob.py();
let circuit_data = ob.getattr("_data")?;
let data_borrowed = circuit_data.extract::<CircuitData>()?;
Ok(QuantumCircuitData {
data: data_borrowed,
name: ob.getattr(intern!(py, "name")).ok(),
calibrations: ob.getattr(intern!(py, "calibrations"))?.extract().ok(),
metadata: ob.getattr(intern!(py, "metadata")).ok(),
qregs: ob
.getattr(intern!(py, "qregs"))
.map(|ob| ob.downcast_into())?
.ok(),
cregs: ob
.getattr(intern!(py, "cregs"))
.map(|ob| ob.downcast_into())?
.ok(),
input_vars: ob
.call_method0(intern!(py, "iter_input_vars"))?
.iter()?
.collect::<PyResult<Vec<_>>>()?,
captured_vars: ob
.call_method0(intern!(py, "iter_captured_vars"))?
.iter()?
.collect::<PyResult<Vec<_>>>()?,
declared_vars: ob
.call_method0(intern!(py, "iter_declared_vars"))?
.iter()?
.collect::<PyResult<Vec<_>>>()?,
})
}
}

#[pyfunction(signature = (quantum_circuit, copy_operations = true, qubit_order = None, clbit_order = None))]
pub fn circuit_to_dag(
py: Python,
quantum_circuit: QuantumCircuitData,
copy_operations: bool,
qubit_order: Option<Vec<Bound<PyAny>>>,
clbit_order: Option<Vec<Bound<PyAny>>>,
) -> PyResult<DAGCircuit> {
DAGCircuit::from_circuit(
py,
quantum_circuit,
copy_operations,
qubit_order,
clbit_order,
)
}

pub fn converters(m: &Bound<PyModule>) -> PyResult<()> {
m.add_function(wrap_pyfunction!(circuit_to_dag, m)?)?;
Ok(())
}
208 changes: 208 additions & 0 deletions crates/circuit/src/dag_circuit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,11 @@ use std::hash::{Hash, Hasher};
use ahash::RandomState;

use crate::bit_data::BitData;
use crate::circuit_data::CircuitData;
use crate::circuit_instruction::{
CircuitInstruction, ExtraInstructionAttributes, OperationFromPython,
};
use crate::converters::QuantumCircuitData;
use crate::dag_node::{DAGInNode, DAGNode, DAGOpNode, DAGOutNode};
use crate::dot_utils::build_dot;
use crate::error::DAGCircuitError;
Expand Down Expand Up @@ -6562,7 +6564,12 @@ impl DAGCircuit {
predecessor_node
};

// Because `DAGCircuit::additional_wires` can return repeated instances of vars,
// we need to make sure to skip those to avoid cycles.
vars_last_nodes.set_item(var, new_node.index())?;
if var_last_node == new_node {
continue;
}
self.dag
.add_edge(var_last_node, new_node, Wire::Var(var.clone_ref(py)));
}
Expand Down Expand Up @@ -6590,6 +6597,207 @@ impl DAGCircuit {

Ok(new_nodes)
}

/// Alternative constructor to build an instance of [DAGCircuit] from a `QuantumCircuit`.
pub(crate) fn from_circuit(
py: Python,
qc: QuantumCircuitData,
copy_op: bool,
qubit_order: Option<Vec<Bound<PyAny>>>,
clbit_order: Option<Vec<Bound<PyAny>>>,
) -> PyResult<DAGCircuit> {
// Extract necessary attributes
let qc_data = qc.data;
let num_qubits = qc_data.num_qubits();
let num_clbits = qc_data.num_clbits();
let num_ops = qc_data.__len__();
Copy link
Member

Choose a reason for hiding this comment

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

Heh, I feel like we should have a native len() method to CircuitData. Not for this PR of course, it's just a bit weird to see this in rust code.

let num_vars = qc.declared_vars.len() + qc.input_vars.len() + qc.captured_vars.len();

// Build DAGCircuit with capacity
let mut new_dag = DAGCircuit::with_capacity(
py,
num_qubits,
num_clbits,
Some(num_vars),
Some(num_ops),
None,
)?;

// Assign other necessary data
new_dag.name = qc.name.map(|ob| ob.unbind());

// Avoid manually acquiring the GIL.
new_dag.global_phase = match qc_data.global_phase() {
Param::ParameterExpression(exp) => Param::ParameterExpression(exp.clone_ref(py)),
Param::Float(float) => Param::Float(*float),
_ => unreachable!("Incorrect parameter assigned for global phase"),
};

if let Some(calibrations) = qc.calibrations {
new_dag.calibrations = calibrations;
}

new_dag.metadata = qc.metadata.map(|meta| meta.unbind());

// Add the qubits depending on order.
let qubit_map: Option<Vec<Qubit>> = if let Some(qubit_ordering) = qubit_order {
let mut ordered_vec = Vec::from_iter((0..num_qubits as u32).map(Qubit));
qubit_ordering
.into_iter()
.try_for_each(|qubit| -> PyResult<()> {
if new_dag.qubits.find(&qubit).is_some() {
return Err(DAGCircuitError::new_err(format!(
"duplicate qubits {}",
&qubit
)));
}
let qubit_index = qc_data.qubits().find(&qubit).unwrap();
ordered_vec[qubit_index.0 as usize] =
new_dag.add_qubit_unchecked(py, &qubit)?;
Ok(())
})?;
Some(ordered_vec)
} else {
qc_data
.qubits()
.bits()
.iter()
.try_for_each(|qubit| -> PyResult<_> {
new_dag.add_qubit_unchecked(py, qubit.bind(py))?;
Ok(())
})?;
None
};

// Add the clbits depending on order.
let clbit_map: Option<Vec<Clbit>> = if let Some(clbit_ordering) = clbit_order {
let mut ordered_vec = Vec::from_iter((0..num_clbits as u32).map(Clbit));
clbit_ordering
.into_iter()
.try_for_each(|clbit| -> PyResult<()> {
if new_dag.clbits.find(&clbit).is_some() {
return Err(DAGCircuitError::new_err(format!(
"duplicate clbits {}",
&clbit
)));
};
let clbit_index = qc_data.clbits().find(&clbit).unwrap();
ordered_vec[clbit_index.0 as usize] =
new_dag.add_clbit_unchecked(py, &clbit)?;
Ok(())
})?;
Some(ordered_vec)
} else {
qc_data
.clbits()
.bits()
.iter()
.try_for_each(|clbit| -> PyResult<()> {
new_dag.add_clbit_unchecked(py, clbit.bind(py))?;
Ok(())
})?;
None
};

// Add all of the new vars.
for var in &qc.declared_vars {
new_dag.add_var(py, var, DAGVarType::Declare)?;
}

for var in &qc.input_vars {
new_dag.add_var(py, var, DAGVarType::Input)?;
}

for var in &qc.captured_vars {
new_dag.add_var(py, var, DAGVarType::Capture)?;
}

// Add all the registers
if let Some(qregs) = qc.qregs {
for qreg in qregs.iter() {
new_dag.add_qreg(py, &qreg)?;
}
}

if let Some(cregs) = qc.cregs {
for creg in cregs.iter() {
new_dag.add_creg(py, &creg)?;
}
}

// Pre-process and re-intern all indices again.
let instructions: Vec<PackedInstruction> = qc_data
.iter()
.map(|instr| -> PyResult<PackedInstruction> {
// Re-map the qubits
let new_qargs = if let Some(qubit_mapping) = &qubit_map {
let qargs = qc_data
.get_qargs(instr.qubits)
.iter()
.map(|bit| qubit_mapping[bit.0 as usize])
.collect();
new_dag.qargs_interner.insert_owned(qargs)
} else {
new_dag
.qargs_interner
.insert(qc_data.get_qargs(instr.qubits))
};
// Remap the clbits
let new_cargs = if let Some(clbit_mapping) = &clbit_map {
let qargs = qc_data
.get_cargs(instr.clbits)
.iter()
.map(|bit| clbit_mapping[bit.0 as usize])
.collect();
new_dag.cargs_interner.insert_owned(qargs)
} else {
new_dag
.cargs_interner
.insert(qc_data.get_cargs(instr.clbits))
};
// Copy the operations

Ok(PackedInstruction {
op: if copy_op {
instr.op.py_deepcopy(py, None)?
} else {
instr.op.clone()
},
qubits: new_qargs,
clbits: new_cargs,
params: instr.params.clone(),
extra_attrs: instr.extra_attrs.clone(),
#[cfg(feature = "cache_pygates")]
py_op: OnceCell::new(),
})
})
.collect::<PyResult<Vec<_>>>()?;

// Finally add all the instructions back
new_dag.extend(py, instructions)?;
Comment on lines +6729 to +6777
Copy link
Member

Choose a reason for hiding this comment

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

Is the only reason you're collecting into a vec here because the deepcopy() call is fallible. I was kind of excited to have this all work with iterators in rust so we avoided a big allocation like this. I don't think we want to mask the error in python if a deepcopy fails, that is important for users to get the python exception. While it ends up being a big block of duplicated code, do you think it would make sense to move the copy_op conditional outside the iterator so you do something like:

if copy_op {
   let instructions = iterator.... .collect::<PyResult<Vec<_>>>()
} else {
   new_dag.extend(py, iterator)
}

where iterator is basically this entire chain just with or without the pyresult depending on whether we call clone() or deepcopy.

Copy link
Contributor Author

@raynelfss raynelfss Sep 10, 2024

Choose a reason for hiding this comment

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

The other reason we're re-collecting is to re-intern the qargs. But I also believe this could be avoided. After all the only requirement here is that whatever we send to extend() can turn into an iterator of PackedInstruction. I think we can avoid the allocation by not collecting the Vec but we would still have to re-intern things since we're not copying the interners over.

Edit: This earlier suggestion may not work due to the iterator mutably borrowing the dag while the method already holds right to mutate. :/

Copy link
Member

Choose a reason for hiding this comment

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

I think we can do this without re-interning at all in the case when we're not reordering any qubits or clbits. Since the interners should be the same for the dag and circuit.

But, lets save this as a potential followup then. The gains from doing something like this would be be measurable but even with a collection into a Vec this has a good improvement in speed and the logic to figure out how to do this is probably will probably be pretty verbose to handle all the edge cases around whether we are reordering any bits or not, or we need to return a python exception from deepcopy or not. I'll open an issue after this merges so we can track it as a potential future optimization.


Ok(new_dag)
}

/// Builds a [DAGCircuit] based on an instance of [CircuitData].
pub fn from_circuit_data(
py: Python,
circuit_data: CircuitData,
copy_op: bool,
) -> PyResult<Self> {
let circ = QuantumCircuitData {
data: circuit_data,
name: None,
calibrations: None,
metadata: None,
qregs: None,
cregs: None,
input_vars: Vec::new(),
captured_vars: Vec::new(),
declared_vars: Vec::new(),
};
Self::from_circuit(py, circ, copy_op, None, None)
}
}

/// Add to global phase. Global phase can only be Float or ParameterExpression so this
Expand Down
1 change: 1 addition & 0 deletions crates/circuit/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
pub mod bit_data;
pub mod circuit_data;
pub mod circuit_instruction;
pub mod converters;
pub mod dag_circuit;
pub mod dag_node;
mod dot_utils;
Expand Down
1 change: 1 addition & 0 deletions crates/pyext/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ where
#[pymodule]
fn _accelerate(m: &Bound<PyModule>) -> PyResult<()> {
add_submodule(m, qiskit_circuit::circuit, "circuit")?;
add_submodule(m, qiskit_circuit::converters::converters, "converters")?;
add_submodule(m, qiskit_qasm2::qasm2, "qasm2")?;
add_submodule(m, qiskit_qasm3::qasm3, "qasm3")?;
add_submodule(m, circuit_library, "circuit_library")?;
Expand Down
1 change: 1 addition & 0 deletions qiskit/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@
# and not have to rely on attribute access. No action needed for top-level extension packages.
sys.modules["qiskit._accelerate.circuit"] = _accelerate.circuit
sys.modules["qiskit._accelerate.circuit_library"] = _accelerate.circuit_library
sys.modules["qiskit._accelerate.converters"] = _accelerate.converters
sys.modules["qiskit._accelerate.convert_2q_block_matrix"] = _accelerate.convert_2q_block_matrix
sys.modules["qiskit._accelerate.dense_layout"] = _accelerate.dense_layout
sys.modules["qiskit._accelerate.error_map"] = _accelerate.error_map
Expand Down
Loading
Loading