From c034e51a209ce9d205f7f4e77567814b2ff3ff84 Mon Sep 17 00:00:00 2001 From: Raynel Sanchez <87539502+raynelfss@users.noreply.github.com> Date: Fri, 21 Feb 2025 14:00:51 -0500 Subject: [PATCH] FIx: Streamline constructors for `Register` - Add missing quotation marks in `Register` repr(). --- crates/circuit/src/bit.rs | 73 ++++++------ crates/circuit/src/register.rs | 206 ++++++++++++++++----------------- 2 files changed, 142 insertions(+), 137 deletions(-) diff --git a/crates/circuit/src/bit.rs b/crates/circuit/src/bit.rs index 39aea7255a6..eeafe9c7991 100644 --- a/crates/circuit/src/bit.rs +++ b/crates/circuit/src/bit.rs @@ -225,25 +225,7 @@ impl PyBit { #[new] #[pyo3(signature = (register = None, index = None))] fn new(register: Option, index: Option) -> PyResult { - match (register, index) { - (Some(register), Some(index)) => { - let RegisterInfo::Owning(owned) = register.0.as_ref() else { - return Err(CircuitError::new_err( - "The provided register for this bit was invalid.", - )); - }; - if index as usize >= owned.len() { - return Err(CircuitError::new_err(format!( - "index must be under the size of the register: {index} was provided" - ))); - } - Ok(Self(BitInfo::new_owned(owned.clone(), index, None))) - } - (None, None) => Ok(Self(BitInfo::new_anonymous(None))), - _ => Err(CircuitError::new_err( - "You should provide both a valid register and an index, not either or.".to_string(), - )), - } + Self::inner_new(register, index, None) } pub fn __repr__(slf: Bound) -> PyResult { @@ -338,23 +320,45 @@ impl PyBit { } } -// impl PyBit { -// /// Quickly retrieves the inner `BitData` living in the `Bit` -// pub(crate) fn inner_bit_info(&self) -> &BitInfo { -// &self.0 -// } -// } +impl PyBit { + /// Quickly retrieves the inner `BitData` living in the `Bit` + pub(crate) fn inner_new( + register: Option, + index: Option, + extra: Option, + ) -> PyResult { + match (register, index) { + (Some(register), Some(index)) => { + let RegisterInfo::Owning(owned) = register.0.as_ref() else { + return Err(CircuitError::new_err( + "The provided register for this bit was invalid.", + )); + }; + if index as usize >= owned.len() { + return Err(CircuitError::new_err(format!( + "index must be under the size of the register: {index} was provided" + ))); + } + Ok(Self(BitInfo::new_owned(owned.clone(), index, extra))) + } + (None, None) => Ok(Self(BitInfo::new_anonymous(extra))), + _ => Err(CircuitError::new_err( + "You should provide both a valid register and an index, not either or.".to_string(), + )), + } + } +} macro_rules! create_py_bit { ($name:ident, $natbit:tt, $pyname:literal, $pymodule:literal, $extra:expr, $pyreg:tt) => { /// Implements a quantum bit #[pyclass( - subclass, - name = $pyname, - module = $pymodule, - frozen, - extends=PyBit, - )] + subclass, + name = $pyname, + module = $pymodule, + frozen, + extends=PyBit, + )] #[derive(Debug, Clone, PartialEq, Eq, Hash)] pub struct $name(pub(crate) $natbit); @@ -364,9 +368,10 @@ macro_rules! create_py_bit { #[new] #[pyo3(signature = (register = None, index = None))] fn new(register: Option<$pyreg>, index: Option) -> PyResult<(Self, PyBit)> { - let inner = PyBit::new( + let inner = PyBit::inner_new( register.clone().map(|reg| PyRegister(reg.data().clone())), index, + Some($extra), )?; Ok((Self($natbit(inner.0.clone())), inner)) } @@ -401,7 +406,7 @@ create_py_bit!( ShareableQubit, "Qubit", "qiskit.circuit.quantumcircuit", - QubitExtraInfo { is_ancilla: false }, + BitExtraInfo::Qubit { is_ancilla: false }, PyQuantumRegister ); @@ -431,7 +436,7 @@ create_py_bit!( ShareableClbit, "Clbit", "qiskit.circuit.classicalregister", - (), + BitExtraInfo::Clbit(), PyClassicalRegister ); diff --git a/crates/circuit/src/register.rs b/crates/circuit/src/register.rs index ba8657a8939..c58588cdabe 100644 --- a/crates/circuit/src/register.rs +++ b/crates/circuit/src/register.rs @@ -22,7 +22,7 @@ use indexmap::IndexSet; use pyo3::{ exceptions::PyValueError, prelude::*, - types::{PyList, PyType}, + types::{PyList, PyString, PyType}, IntoPyObjectExt, PyTypeInfo, }; use std::{ @@ -353,7 +353,7 @@ impl PyRegister { pub fn new( size: Option>, name: Option>, - bits: Option>, + bits: Option>, ) -> PyResult { let name_parse = |name: Option| -> String { if let Some(name) = name { @@ -366,42 +366,7 @@ impl PyRegister { } }; - if (size.is_none() && bits.is_none()) || (size.is_some() && bits.is_some()) { - return Err(CircuitError::new_err(format!("Exactly one of the size or bits arguments can be provided. Provided size={:?} bits={:?}.", size, bits))); - } - - let size: u32 = if let Some(bits) = bits.as_ref() { - bits.len().try_into().map_err(|_| CircuitError::new_err(format!("The amount of bits provided exceeds the capacity of the register. Current size {}", bits.len())))? - } else { - let Ok(valid_size): PyResult = size.as_ref().unwrap().extract() else { - return Err(CircuitError::new_err(format!( - "Register size must be an integer. {} '{}' was provided", - size.as_ref().unwrap().get_type().name()?, - size.as_ref().unwrap().repr()? - ))); - }; - if valid_size < 0 { - return Err(CircuitError::new_err(format!( - "Register size must be non-negative. {} '{}' was provided", - size.as_ref().unwrap().get_type().name()?, - size.as_ref().unwrap().repr()? - ))); - } - - let Ok(valid_size) = valid_size.abs().try_into() else { - return Err(CircuitError::new_err(format!( - "Register size exceeds possible allocated capacity. {} '{}' was provided", - size.as_ref().unwrap().get_type().name()?, - size.as_ref().unwrap().repr()? - ))); - }; - - valid_size - }; - - let Ok(name) = name.as_ref().map(|name| name.extract()).transpose() else { - return Err(CircuitError::new_err("The circuit name should be castable to a string (or None for autogenerate a name).")); - }; + let (size, name) = Self::inner_parse_new(&size, &name, &bits)?; let register = if let Some(bits) = bits { let Ok(bits_set): PyResult> = bits @@ -432,13 +397,13 @@ impl PyRegister { let borrowed = slf.borrow(); match borrowed.0.as_ref() { RegisterInfo::Owning(owning_register_info) => Ok(format!( - "{}({}, {})", + "{}({}, '{}')", slf.get_type().qualname()?, owning_register_info.len(), owning_register_info.name() )), RegisterInfo::Alias { name, bits, .. } => Ok(format!( - "{}({}, {})", + "{}({}, '{}')", slf.get_type().qualname()?, bits.len(), name, @@ -582,6 +547,60 @@ impl PyRegister { } impl PyRegister { + /// Correctly performs extraction of size and name during creation of a register + pub fn inner_parse_new( + size: &Option>, + name: &Option>, + bits: &Option>, + ) -> PyResult<(u32, Option)> { + if (size.is_none() && bits.is_none()) || (size.is_some() && bits.is_some()) { + return Err(CircuitError::new_err(format!("Exactly one of the size or bits arguments can be provided. Provided size={:?} bits={:?}.", size, bits))); + } + + let size: u32 = if let Some(bits) = bits.as_ref() { + bits.len()?.try_into().map_err(|_| CircuitError::new_err(format!("The amount of bits provided exceeds the capacity of the register. Current size {}", bits.len().unwrap_or_default())))? + } else { + let Ok(valid_size): PyResult = size.as_ref().unwrap().extract() else { + return Err(CircuitError::new_err(format!( + "Register size must be an integer. {} '{}' was provided", + size.as_ref().unwrap().get_type().name()?, + size.as_ref().unwrap().repr()? + ))); + }; + if valid_size < 0 { + return Err(CircuitError::new_err(format!( + "Register size must be non-negative. {} '{}' was provided", + size.as_ref().unwrap().get_type().name()?, + size.as_ref().unwrap().repr()? + ))); + } + + let Ok(valid_size) = valid_size.abs().try_into() else { + return Err(CircuitError::new_err(format!( + "Register size exceeds possible allocated capacity. {} '{}' was provided", + size.as_ref().unwrap().get_type().name()?, + size.as_ref().unwrap().repr()? + ))); + }; + + valid_size + }; + + let Ok(name) = name + .as_ref() + .map(|name| -> PyResult { + PyString::type_object(name.py()) + .call1((name,))? + .extract::() + }) + .transpose() + else { + return Err(CircuitError::new_err("The circuit name should be castable to a string (or None for autogenerate a name).")); + }; + + Ok((size, name)) + } + /// Inner function for [PyRegister::__getnewargs__] to ensure serialization can be /// preserved between register types. fn getitem_inner( @@ -639,7 +658,6 @@ enum SliceOrInt<'py> { macro_rules! create_py_register { ($name:ident, $nativereg:tt, $pybit:tt, $nativebit:tt, $pyname:literal, $pymodule:literal, $extra:expr, $prefix:literal) => { - #[pyclass( name = $pyname, module = $pymodule, @@ -658,38 +676,22 @@ macro_rules! create_py_register { pub fn new( size: Option>, name: Option>, - bits: Option>, + bits: Option>, ) -> PyResult<(Self, PyRegister)> { - if (size.is_none() && bits.is_none()) || (size.is_some() && bits.is_some()) { - return Err(CircuitError::new_err(format!("Exactly one of the size or bits arguments can be provided. Provided size={:?} bits={:?}.", size, bits))); - } - - let size: u32 = if let Some(bits) = bits.as_ref() { - bits.len().try_into().map_err(|_| CircuitError::new_err(format!("The amount of bits provided exceeds the capacity of the register. Current size {}", bits.len())))? - } else { - let Ok(valid_size) : PyResult = size.as_ref().unwrap().extract() else { - return Err(CircuitError::new_err(format!("Register size must be an integer. {} '{}' was provided", size.as_ref().unwrap().get_type().name()?, size.as_ref().unwrap().repr()?))) - }; - if valid_size < 0 { - return Err(CircuitError::new_err(format!("Register size must be non-negative. {} '{}' was provided", size.as_ref().unwrap().get_type().name()?, size.as_ref().unwrap().repr()?))) - } - - let Ok(valid_size) = valid_size.abs().try_into() else { - return Err(CircuitError::new_err(format!("Register size exceeds possible allocated capacity. {} '{}' was provided", size.as_ref().unwrap().get_type().name()?, size.as_ref().unwrap().repr()?))) - }; - - valid_size - }; - - let Ok(name) = name.as_ref().map(|name| name.extract()).transpose() else { - return Err(CircuitError::new_err("The circuit name should be castable to a string (or None for autogenerate a name).")) - }; + let (size, name) = PyRegister::inner_parse_new(&size, &name, &bits)?; let register = if let Some(bits) = bits { - let Ok(bits_set) : PyResult> = bits.try_iter()?.map(|bit| -> PyResult { - bit?.extract::<$pybit>().map(|b| b.0.0) - }).collect() else { - return Err(CircuitError::new_err(format!("Provided bits did not all match register type. bits={}", bits.repr()?))) + let Ok(bits_set): PyResult> = bits + .try_iter()? + .map(|bit| -> PyResult { + bit?.extract::<$pybit>().map(|b| b.0 .0) + }) + .collect() + else { + return Err(CircuitError::new_err(format!( + "Provided bits did not all match register type. bits={}", + bits.repr()? + ))); }; if bits_set.len() != size as usize { return Err(CircuitError::new_err( @@ -711,24 +713,17 @@ macro_rules! create_py_register { let sequence = py_sequence_index .with_len(slf.as_super().size().try_into().unwrap())?; match sequence { - crate::slice::SequenceIndex::Int(_) => slf - .getitem_inner(key)? - .next() - .into_py_any(py), - _ => Ok(PyList::new( - py, - slf.getitem_inner(key)?, - )? - .into_any() - .unbind()), + crate::slice::SequenceIndex::Int(_) => { + slf.getitem_inner(key)?.next().into_py_any(py) + } + _ => Ok(PyList::new(py, slf.getitem_inner(key)?)? + .into_any() + .unbind()), } } - _ => Ok(PyList::new( - py, - slf.getitem_inner(key)?, - )? - .into_any() - .unbind()), + _ => Ok(PyList::new(py, slf.getitem_inner(key)?)? + .into_any() + .unbind()), } } @@ -737,10 +732,7 @@ macro_rules! create_py_register { } fn __iter__(slf: PyRef<'_, Self>) -> PyResult> { - let list: Vec<$nativebit> = slf - .0 - .bits() - .collect(); + let list: Vec<$nativebit> = slf.0.bits().collect(); list.into_pyobject(slf.py())?.try_iter() } @@ -773,28 +765,36 @@ macro_rules! create_py_register { match sequence { crate::slice::SequenceIndex::Int(idx) => { let Some(bit) = self.0.get(idx) else { - return Err(CircuitError::new_err("register index out of range")); + return Err(CircuitError::new_err( + "register index out of range", + )); }; Ok(Box::new(std::iter::once(bit))) - }, + } _ => { - let result: Vec<$nativebit> = sequence.iter().map( - |idx| -> PyResult<$nativebit> { - self.0.get(idx).ok_or(CircuitError::new_err("register index out of range")) - } - ).collect::>()?; + let result: Vec<$nativebit> = sequence + .iter() + .map(|idx| -> PyResult<$nativebit> { + self.0.get(idx).ok_or(CircuitError::new_err( + "register index out of range", + )) + }) + .collect::>()?; Ok(Box::new(result.into_iter())) } } } SliceOrInt::List(vec) => { - let result: Vec<$nativebit> = vec.iter().map( - |idx| -> PyResult<$nativebit> { - self.0.get(*idx).ok_or(CircuitError::new_err("register index out of range")) - } - ).collect::>()?; + let result: Vec<$nativebit> = vec + .iter() + .map(|idx| -> PyResult<$nativebit> { + self.0 + .get(*idx) + .ok_or(CircuitError::new_err("register index out of range")) + }) + .collect::>()?; Ok(Box::new(result.into_iter())) - }, + } } } }