Skip to content

Commit

Permalink
Add; Default to rust bits in rust circuit-generating functions.
Browse files Browse the repository at this point in the history
- Fix error where rust bits wouldn't be initialized in the cache.
- Provisionally fix error in which rust bits wouldn't have valid mapping to python bits. Bits need to be generated before being able to map them to a rust native index. The current solution implements said changes during the `find_bit` function, in which there is an extra validation step. However, this is certain to consume more resources. The ideal solution would be to generate said mapping when obtaining the Python bit with something similar to a `RefCell`. However, `RefCells` are not thread safe and could cause several mutable references to coexist.
  • Loading branch information
raynelfss committed Feb 2, 2025
1 parent ba8bcf8 commit 7690065
Show file tree
Hide file tree
Showing 2 changed files with 52 additions and 36 deletions.
50 changes: 25 additions & 25 deletions crates/circuit/src/bit_data.rs
Original file line number Diff line number Diff line change
Expand Up @@ -457,7 +457,7 @@ where
let key = &BitAsKey::new(bit);
if let Some(value) = self.indices.get(key).copied() {
Ok(Some(value))
} else if self.indices.len() != self.len() {
} else if self.indices.len() < self.len() {
let py = bit.py();
// TODO: Make sure all bits have been mapped during addition.
// The only case in which bits may not be mapped is when a circuit
Expand Down Expand Up @@ -500,7 +500,7 @@ where

// If the length is different from the stored bits, rebuild cache
let res_as_bound = res.bind(py);
if res_as_bound.len() != self.len() {
if res_as_bound.len() < self.len() {
let current_length = res_as_bound.len();
for index in 0..self.len() {
if index < current_length {
Expand All @@ -509,7 +509,7 @@ where
res_as_bound.append(self.py_get_bit(py, (index as u32).into())?)?
}
}
Ok(res)
Ok(self.cached_py_bits.get().unwrap())
} else {
Ok(res)
}
Expand All @@ -528,9 +528,9 @@ where
.into()
});

// If the length is different from the stored bits, rebuild cache
// If the length is different from the stored registers, rebuild cache
let res_as_bound = res.bind(py);
if res_as_bound.len() != self.len_regs() {
if res_as_bound.len() < self.len_regs() {
let current_length = res_as_bound.len();
for index in 0..self.len_regs() {
if index < current_length {
Expand All @@ -539,7 +539,14 @@ where
res_as_bound.append(self.py_get_register(py, index as u32)?)?
}
}
Ok(res)
let trimmed = res_as_bound.get_slice(0, self.len_regs()).unbind();
self.cached_py_regs.set(trimmed).map_err(|_| {
PyRuntimeError::new_err(format!(
"Tried to initialized {} register cache while another thread was initializing",
self.description
))
})?;
Ok(self.cached_py_regs.get().unwrap())
} else {
Ok(res)
}
Expand Down Expand Up @@ -591,21 +598,15 @@ where
/// Map the provided Python bits to their native indices.
/// An error is returned if any bit is not registered.
pub fn py_map_bits<'py>(
&self,
&mut self,
bits: impl IntoIterator<Item = Bound<'py, PyAny>>,
) -> PyResult<impl Iterator<Item = T>> {
let v: Result<Vec<_>, _> = bits
.into_iter()
.map(|b| {
self.indices
.get(&BitAsKey::new(&b))
.copied()
.ok_or_else(|| {
PyKeyError::new_err(format!(
"Bit {:?} has not been added to this circuit.",
b
))
})
self.py_find_bit(&b)?.ok_or_else(|| {
PyKeyError::new_err(format!("Bit {:?} has not been added to this circuit.", b))
})
})
.collect();
v.map(|x| x.into_iter())
Expand Down Expand Up @@ -646,12 +647,9 @@ where
self.bits[index_as_usize]
.set(res.into())
.map_err(|_| PyRuntimeError::new_err("Could not set the OnceCell correctly"))?;
return Ok(self.bits[index_as_usize].get());
}
// If it is initialized, just retrieve.
else {
return Ok(self.bits[index_as_usize].get());
}
Ok(self.bits[index_as_usize].get())
} else if let Some(bit) = self.bits[index_as_usize].get() {
Ok(Some(bit))
} else {
Expand Down Expand Up @@ -746,13 +744,15 @@ where
self.py_cached_bits(py)?.bind(py).append(bit)?;
self.bit_info.push(BitInfo::new(None));
self.bits.push(bit.clone().unbind().into());
Ok(idx.into())
} else if strict {
return Err(PyValueError::new_err(format!(
"Existing bit {:?} cannot be re-added in strict mode.",
bit
)));
} else {
return self.py_find_bit(bit).map(|opt| opt.unwrap());
}
Ok(idx.into())
}

/// Adds new register from Python.
Expand Down Expand Up @@ -791,10 +791,10 @@ where
))
})?;
let bit = bit?;
let index = if let Some(index) = self.indices.get(&BitAsKey::new(&bit)) {
let bit_info = &mut self.bit_info[BitType::from(*index) as usize];
let index = if let Some(index) = self.py_find_bit(&bit)? {
let bit_info = &mut self.bit_info[BitType::from(index) as usize];
bit_info.add_register(idx, bit_index);
*index
index
} else {
let index = self.py_add_bit(&bit, true)?;
self.bit_info[BitType::from(index) as usize] =
Expand All @@ -809,7 +809,7 @@ where
let name: String = key.name().to_string();
let reg: R = (bits.as_slice(), name).into();

// Append to cache before bits to avoid rebuilding cache.
// Append to cache before registers to avoid rebuilding cache.
self.py_cached_regs(py)?.bind(py).append(register)?;
self.reg_keys.insert(reg.as_key().clone(), idx);
self.registry.push(reg);
Expand Down
38 changes: 27 additions & 11 deletions crates/circuit/src/circuit_data.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ use crate::circuit_instruction::{
CircuitInstruction, ExtraInstructionAttributes, OperationFromPython,
};
use crate::dag_circuit::add_global_phase;
use crate::imports::{ANNOTATED_OPERATION, CLBIT, QUANTUM_CIRCUIT, QUBIT};
use crate::imports::{ANNOTATED_OPERATION, QUANTUM_CIRCUIT};
use crate::interner::{Interned, Interner};
use crate::operations::{Operation, OperationRef, Param, StandardGate};
use crate::packed_instruction::{PackedInstruction, PackedOperation};
Expand Down Expand Up @@ -1074,8 +1074,11 @@ impl CircuitData {
if add_creg { 1 } else { 0 },
),
param_table: ParameterTable::new(),
global_phase,
global_phase: Param::Float(0.),
};

// Set the global phase using internal setter.
data._set_global_phase_float(global_phase);
// Add all the bits into a register
if add_qreg {
data.add_qreg(
Expand Down Expand Up @@ -1136,7 +1139,7 @@ impl CircuitData {
(0..self.qubits.len_regs()).flat_map(|index| self.qubits.get_register(index as u32))
}

/// Adds a generic qubit to a circuit
/// Adds a generic clbit to a circuit
pub fn add_clbit(&mut self) -> Clbit {
self.clbits.add_bit()
}
Expand All @@ -1161,6 +1164,23 @@ impl CircuitData {
self.clbits.get_bit_info(clbit)
}

/// Set the global phase of the circuit using a float, without needing a
/// `py` token.
///
/// _**Note:** for development purposes only. Should be removed after
/// [#13278](https://github.com/Qiskit/qiskit/pull/13278)._
fn _set_global_phase_float(&mut self, angle: Param) {
match angle {
Param::Float(angle) => {
self.global_phase = Param::Float(angle.rem_euclid(2. * std::f64::consts::PI));
}
_ => panic!(
"Could not set the parameter {:?}. Parameter was not a float.",
&angle
),
}
}

/// An alternate constructor to build a new `CircuitData` from an iterator
/// of packed operations. This can be used to build a circuit from a sequence
/// of `PackedOperation` without needing to involve Python.
Expand Down Expand Up @@ -1350,8 +1370,8 @@ impl CircuitData {
data: Vec::with_capacity(instruction_capacity),
qargs_interner: Interner::new(),
cargs_interner: Interner::new(),
qubits: NewBitData::new("qubits".to_string()),
clbits: NewBitData::new("clbits".to_string()),
qubits: NewBitData::with_capacity("qubits".to_string(), num_qubits as usize, 0),
clbits: NewBitData::with_capacity("clbits".to_string(), num_clbits as usize, 0),
param_table: ParameterTable::new(),
global_phase: Param::Float(0.0),
};
Expand All @@ -1361,17 +1381,13 @@ impl CircuitData {
res.set_global_phase(py, global_phase)?;

if num_qubits > 0 {
let qubit_cls = QUBIT.get_bound(py);
for _i in 0..num_qubits {
let bit = qubit_cls.call0()?;
res.py_add_qubit(&bit, true)?;
res.add_qubit();
}
}
if num_clbits > 0 {
let clbit_cls = CLBIT.get_bound(py);
for _i in 0..num_clbits {
let bit = clbit_cls.call0()?;
res.py_add_clbit(&bit, true)?;
res.add_clbit();
}
}
Ok(res)
Expand Down

0 comments on commit 7690065

Please sign in to comment.