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.

Sign up for GitHub

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

Add new API for inserting multiple operations into the DAGCircuit in Rust. #13335

Open
wants to merge 14 commits into
base: main
Choose a base branch
from
Open

Add new API for inserting multiple operations into the DAGCircuit in Rust. #13335

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
85b3bc3
Initial: Add `DAGCircuitConcat` struct and new insertion API.
raynelfss Oct 16, 2024
b87e528
Merge branch 'main' into dag-concatenate
raynelfss Oct 16, 2024
858e044
Merge branch 'main' into dag-concatenate
raynelfss Oct 16, 2024
ffb1d28
Add: `OwnedOrSlice<T>`
raynelfss Oct 18, 2024
3d776fa
Merge branch 'main' into dag-concatenate
raynelfss Oct 18, 2024
2dab9c9
Merge branch 'main' into dag-concatenate
raynelfss Oct 18, 2024
2776bd4
Add: Optional qargs/cargs in `apply_operation_back`
raynelfss Oct 18, 2024
1bbc504
Merge branch 'main' into dag-concatenate
raynelfss Oct 31, 2024
c91d45b
Fix: Leverage usage in `BasisTranslator`
raynelfss Oct 31, 2024
b67a441
Merge remote-tracking branch 'upstream/main' into dag-concatenate
raynelfss Nov 21, 2024
572dd11
Merge remote-tracking branch 'upstream/main' into dag-concatenate
raynelfss Jan 23, 2025
cc4f1dc
Fix: Replace `OnceCell` with `OnceLock`
raynelfss Jan 23, 2025
6708d00
Merge remote-tracking branch 'upstream/main' into dag-concatenate
raynelfss Jan 24, 2025
4bac9c2
Fix: Remove `SliceOrOwned` in favor of `Cow`
raynelfss Jan 24, 2025
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
55 changes: 29 additions & 26 deletions crates/accelerate/src/basis/basis_translator/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ use pyo3::types::{IntoPyDict, PyComplex, PyDict, PyTuple};
use pyo3::PyTypeInfo;
use qiskit_circuit::circuit_instruction::OperationFromPython;
use qiskit_circuit::converters::circuit_to_dag;
use qiskit_circuit::dag_circuit::DAGCircuitConcat;
use qiskit_circuit::imports::DAG_TO_CIRCUIT;
use qiskit_circuit::imports::PARAMETER_EXPRESSION;
use qiskit_circuit::operations::Param;
Expand Down Expand Up @@ -427,6 +428,7 @@ fn apply_translation(
) -> PyResult<(DAGCircuit, bool)> {
let mut is_updated = false;
let mut out_dag = dag.copy_empty_like(py, "alike")?;
let mut out_dag_concat = out_dag.as_concat();
for node in dag.topological_op_nodes()? {
let node_obj = dag[node].unwrap_operation();
let node_qarg = dag.get_qargs(node_obj.qubits);
Expand Down Expand Up @@ -469,11 +471,11 @@ fn apply_translation(
new_op = Some(replaced_blocks.extract()?);
}
if let Some(new_op) = new_op {
out_dag.apply_operation_back(
out_dag_concat.apply_operation_back(
py,
new_op.operation,
node_qarg,
node_carg,
Some(node_qarg.into()),
Some(node_carg.into()),
if new_op.params.is_empty() {
None
} else {
Expand All @@ -482,13 +484,13 @@ fn apply_translation(
new_op.extra_attrs,
#[cfg(feature = "cache_pygates")]
None,
)?;
);
} else {
out_dag.apply_operation_back(
out_dag_concat.apply_operation_back(
py,
node_obj.op.clone(),
node_qarg,
node_carg,
Some(node_qarg.into()),
Some(node_carg.into()),
if node_obj.params_view().is_empty() {
None
} else {
Expand All @@ -503,7 +505,7 @@ fn apply_translation(
node_obj.extra_attrs.clone(),
#[cfg(feature = "cache_pygates")]
None,
)?;
);
}
continue;
}
Expand All @@ -512,11 +514,11 @@ fn apply_translation(
if qargs_with_non_global_operation.contains_key(&node_qarg_as_physical)
&& qargs_with_non_global_operation[&node_qarg_as_physical].contains(node_obj.op.name())
{
out_dag.apply_operation_back(
out_dag_concat.apply_operation_back(
py,
node_obj.op.clone(),
node_qarg,
node_carg,
Some(node_qarg.into()),
Some(node_carg.into()),
if node_obj.params_view().is_empty() {
None
} else {
Expand All @@ -531,16 +533,16 @@ fn apply_translation(
node_obj.extra_attrs.clone(),
#[cfg(feature = "cache_pygates")]
None,
)?;
);
continue;
}

if dag.has_calibration_for_index(py, node)? {
out_dag.apply_operation_back(
out_dag_concat.apply_operation_back(
py,
node_obj.op.clone(),
node_qarg,
node_carg,
Some(node_qarg.into()),
Some(node_carg.into()),
if node_obj.params_view().is_empty() {
None
} else {
Expand All @@ -555,7 +557,7 @@ fn apply_translation(
node_obj.extra_attrs.clone(),
#[cfg(feature = "cache_pygates")]
None,
)?;
);
continue;
}
let unique_qargs: Option<Qargs> = if qubit_set.is_empty() {
Expand All @@ -566,14 +568,14 @@ fn apply_translation(
if extra_inst_map.contains_key(&unique_qargs) {
replace_node(
py,
&mut out_dag,
&mut out_dag_concat,
node_obj.clone(),
&extra_inst_map[&unique_qargs],
)?;
} else if instr_map
.contains_key(&(node_obj.op.name().to_string(), node_obj.op.num_qubits()))
{
replace_node(py, &mut out_dag, node_obj.clone(), instr_map)?;
replace_node(py, &mut out_dag_concat, node_obj.clone(), instr_map)?;
} else {
return Err(TranspilerError::new_err(format!(
"BasisTranslator did not map {}",
Expand All @@ -582,13 +584,14 @@ fn apply_translation(
}
is_updated = true;
}

// Kill the concatenator
out_dag_concat.end();
Ok((out_dag, is_updated))
}

fn replace_node(
py: Python,
dag: &mut DAGCircuit,
dag: &mut DAGCircuitConcat,
node: PackedInstruction,
instr_map: &HashMap<GateIdentifier, (SmallVec<[Param; 3]>, DAGCircuit)>,
) -> PyResult<()> {
Expand Down Expand Up @@ -648,8 +651,8 @@ fn replace_node(
dag.apply_operation_back(
py,
new_op,
&new_qubits,
&new_clbits,
Some(new_qubits.into()),
Some(new_clbits.into()),
if new_params.is_empty() {
None
} else {
Expand All @@ -658,7 +661,7 @@ fn replace_node(
new_extra_props,
#[cfg(feature = "cache_pygates")]
None,
)?;
);
}
dag.add_global_phase(py, target_dag.global_phase())?;
} else {
Expand Down Expand Up @@ -752,8 +755,8 @@ fn replace_node(
dag.apply_operation_back(
py,
new_op,
&new_qubits,
&new_clbits,
Some(new_qubits.into()),
Some(new_clbits.into()),
if new_params.is_empty() {
None
} else {
Expand All @@ -762,7 +765,7 @@ fn replace_node(
inner_node.extra_attrs.clone(),
#[cfg(feature = "cache_pygates")]
None,
)?;
);
}

if let Param::ParameterExpression(old_phase) = target_dag.global_phase() {
Expand Down
65 changes: 45 additions & 20 deletions crates/accelerate/src/unitary_synthesis.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -111,16 +111,31 @@ fn apply_synth_dag(
out_qargs: &[Qubit],
synth_dag: &DAGCircuit,
) -> PyResult<()> {
let mut out_dag_concat = out_dag.as_concat();
for out_node in synth_dag.topological_op_nodes()? {
Comment on lines +114 to 115
Copy link
Member

Choose a reason for hiding this comment

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

This function (apply_synth_dag) doesn't feel like it's high enough in the call tree, because we're allocating and de-allocating the concatenator for each synthesised unitary matrix, whereas in an ideal world we'd do that only once. Perhaps we should be passing the concatenator through the function calls?

let mut out_packed_instr = synth_dag[out_node].unwrap_operation().clone();
let out_packed_instr = synth_dag.dag()[out_node].unwrap_operation();
let synth_qargs = synth_dag.get_qargs(out_packed_instr.qubits);
let mapped_qargs: Vec<Qubit> = synth_qargs
.iter()
.map(|qarg| out_qargs[qarg.0 as usize])
.collect();
out_packed_instr.qubits = out_dag.qargs_interner.insert(&mapped_qargs);
out_dag.push_back(py, out_packed_instr)?;
out_dag_concat.apply_operation_back(
py,
out_packed_instr.op.clone(),
Some(mapped_qargs.into()),
Some(
synth_dag
.cargs_interner()
.get(out_packed_instr.clbits)
.into(),
),
out_packed_instr.params.clone().map(|param| *param),
out_packed_instr.extra_attrs.clone(),
#[cfg(feature = "cache_pygates")]
None,
);
Comment on lines -115 to +136
Copy link
Member

Choose a reason for hiding this comment

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

On line 116: the .dag() call is not necessary any more (probably happened in a merge conflict).

For the whole thing: the resulting code here has ended up a lot more complex than the previous code - perhaps this was before you added the push_back method to your new struct? I'd consider swapping back, even if you need a bit of extra boilerplate to expose the interners in the same way (or at least the relevant Interner methods).

}
out_dag_concat.end();
out_dag.add_global_phase(py, &synth_dag.get_global_phase())?;
Ok(())
}
Expand All @@ -131,29 +146,29 @@ fn apply_synth_sequence(
out_qargs: &[Qubit],
sequence: &TwoQubitUnitarySequence,
) -> PyResult<()> {
let mut instructions = Vec::with_capacity(sequence.gate_sequence.gates().len());
let mut concat_dag = out_dag.as_concat();
for (gate, params, qubit_ids) in sequence.gate_sequence.gates() {
let gate_node = match gate {
None => sequence.decomp_gate.operation.standard_gate(),
Some(gate) => *gate,
};
let mapped_qargs: Vec<Qubit> = qubit_ids.iter().map(|id| out_qargs[*id as usize]).collect();
let new_params: Option<Box<SmallVec<[Param; 3]>>> = match gate {
Some(_) => Some(Box::new(params.iter().map(|p| Param::Float(*p)).collect())),
None => Some(Box::new(sequence.decomp_gate.params.clone())),
let new_params: Option<SmallVec<[Param; 3]>> = match gate {
Some(_) => Some(params.iter().map(|p| Param::Float(*p)).collect()),
None => Some(sequence.decomp_gate.params.clone()),
};
let instruction = PackedInstruction {
op: PackedOperation::from_standard(gate_node),
qubits: out_dag.qargs_interner.insert(&mapped_qargs),
clbits: out_dag.cargs_interner.get_default(),
params: new_params,
extra_attrs: ExtraInstructionAttributes::default(),
concat_dag.apply_operation_back(
py,
PackedOperation::from_standard(gate_node),
Some(mapped_qargs.into()),
None,
new_params,
ExtraInstructionAttributes::default(),
#[cfg(feature = "cache_pygates")]
py_op: OnceLock::new(),
};
instructions.push(instruction);
None,
);
}
out_dag.extend(py, instructions.into_iter())?;
concat_dag.end();
out_dag.add_global_phase(py, &Param::Float(sequence.gate_sequence.global_phase()))?;
Ok(())
}
Expand Down Expand Up @@ -1065,17 +1080,27 @@ fn reversed_synth_su4_dag(

let mut target_dag = synth_dag.copy_empty_like(py, "alike")?;
let flip_bits: [Qubit; 2] = [Qubit(1), Qubit(0)];
let mut target_dag_concat = target_dag.as_concat();
for node in synth_dag.topological_op_nodes()? {
let mut inst = synth_dag[node].unwrap_operation().clone();
let inst = synth_dag.dag()[node].unwrap_operation();
let qubits: Vec<Qubit> = synth_dag
.qargs_interner()
.get(inst.qubits)
.iter()
.map(|x| flip_bits[x.0 as usize])
.collect();
inst.qubits = target_dag.qargs_interner.insert_owned(qubits);
target_dag.push_back(py, inst)?;
target_dag_concat.apply_operation_back(
py,
inst.op.clone(),
Some(qubits.into()),
Some(synth_dag.cargs_interner().get(inst.clbits).into()),
inst.params.clone().map(|param| *param),
inst.extra_attrs.clone(),
#[cfg(feature = "cache_pygates")]
None,
);
}
target_dag_concat.end();
Ok(target_dag)
}

Expand Down
Loading
Loading