feat!: Use a common struct for both QPU and QVM execution results

crates/lib/Cargo.toml

@@ -49,3 +49,4 @@ tempfile = "3.3.0"
 tokio = { version = "1.21.2", features = ["macros", "rt-multi-thread"] }
 warp = "0.3.3"
 regex = "1.7.0"
+test-case = "2.2.2"

crates/lib/src/api.rs

@@ -53,7 +53,7 @@ pub enum RewriteArithmeticError {
     Rewrite(#[from] rewrite_arithmetic::Error),
 }
 
-/// The result of a call to [`rewrite_arithmetic`] which provides the
+/// The result of a call to [`rewrite_arithmetic()`] which provides the
 /// information necessary to later patch-in memory values to a compiled program.
 #[derive(Debug, Serialize)]
 pub struct RewriteArithmeticResult {
@@ -112,7 +112,7 @@ pub struct TranslationResult {
 ///
 /// # Errors
 ///
-/// Returns a [`translation::Error`] if translation fails.
+/// Returns a [`TranslationError`] if translation fails.
 pub async fn translate(
     native_quil: &str,
     shots: u16,
@@ -292,7 +292,7 @@ impl From<ControllerJobExecutionResult> for ExecutionResults {
 ///
 /// # Errors
 ///
-/// May error if a [`gRPC`] client cannot be constructed, or a [`gRPC`]
+/// May error if a [`Qcs`] client cannot be constructed, or if the ``gRPC``
 /// call fails.
 pub async fn retrieve_results(
     job_id: &str,

crates/lib/src/executable.rs

@@ -24,7 +24,7 @@ use quil_rs::Program;
 ///
 /// ```rust
 /// use qcs_api_client_common::ClientConfiguration;
-/// use qcs::{Executable, RegisterData};
+/// use qcs::Executable;
 ///
 ///
 /// const PROGRAM: &str = r##"
@@ -40,12 +40,13 @@ use quil_rs::Program;
 /// #[tokio::main]
 /// async fn main() {
 ///     let mut result = Executable::from_quil(PROGRAM).with_config(ClientConfiguration::default()).with_shots(4).execute_on_qvm().await.unwrap();
-///     // We know it's i8 because we declared the memory as `BIT` in Quil.
 ///     // "ro" is the only source read from by default if you don't specify a .read_from()
-///     let data = result.registers.remove("ro").expect("Did not receive ro data").into_i8().unwrap();
-///     // In this case, we ran the program for 4 shots, so we know the length is 4.
-///     assert_eq!(data.len(), 4);
-///     for shot in data {
+///     // We get the data as a shot wise matrix so we can iterate over rows containing all the values in
+///     // in "ro" for each shot.
+///     let data = result.readout_data.get_shot_wise_matrix("ro").expect("should have data in ro");
+///     // In this case, we ran the program for 4 shots, so we know the number of rows is 4.
+///     assert_eq!(data.nrows(), 4);
+///     for shot in data.rows() {
 ///         // Each shot will contain all the memory, in order, for the vector (or "register") we
 ///         // requested the results of. In this case, "ro" (the default).
 ///         assert_eq!(shot.len(), 2);
@@ -143,20 +144,20 @@ impl<'executable> Executable<'executable, '_> {
     ///         .execute_on_qvm()
     ///         .await
     ///         .unwrap();
-    ///     let first = result
-    ///         .registers
-    ///         .remove("first")
-    ///         .expect("Did not receive first buffer")
-    ///         .into_f64()
-    ///         .expect("Received incorrect data type for first");
-    ///     let second = result
-    ///         .registers
-    ///         .remove("second")
-    ///         .expect("Did not receive second buffer")
-    ///         .into_f64()
-    ///         .expect("Received incorrect data type for second");
-    ///     assert_eq!(first[0][0], 3.141);
-    ///     assert_eq!(second[0][0], 1.234);
+    ///     let first_value = result
+    ///         .readout_data
+    ///         .get_value("first", 0, 0)
+    ///         .expect("should have value in first position of first register")
+    ///         .into_real()
+    ///         .expect("should be a floating point number");
+    ///     let second_value = result
+    ///         .readout_data
+    ///         .get_value("second", 0, 0)
+    ///         .expect("should have value in first position of second register")
+    ///         .into_real()
+    ///         .expect("should be a floating point number");
+    ///     assert_eq!(first_value, 3.141);
+    ///     assert_eq!(second_value, 1.234);
     /// }
     /// ```
     #[must_use]
@@ -198,9 +199,20 @@ impl<'executable> Executable<'executable, '_> {
     ///             .with_parameter("theta", 0, theta)
     ///             .with_parameter("theta", 1, theta * 2.0)
     ///             .execute_on_qvm().await.unwrap();
-    ///         let data = result.registers.remove("theta").expect("Could not read theta").into_f64().unwrap();
-    ///         assert_eq!(data[0][0], theta);
-    ///         assert_eq!(data[0][1], theta * 2.0);
+    ///         let first = result
+    ///             .readout_data
+    ///             .get_value("theta", 0, 0)
+    ///             .expect("first index, first shot of theta should have value")
+    ///             .into_real()
+    ///             .expect("value should be a float");
+    ///         let second = result
+    ///             .readout_data
+    ///             .get_value("theta", 1, 0)
+    ///             .expect("second index, first shot of theta should have value")
+    ///             .into_real()
+    ///             .expect("value should be a float");
+    ///         assert_eq!(first, theta);
+    ///         assert_eq!(second, theta * 2.0);
     ///     }
     /// }
     /// ```
@@ -278,7 +290,7 @@ impl Executable<'_, '_> {
     ///
     /// # Returns
     ///
-    /// A `HashMap<String, ExecutionResult>` where the key is the name of the register that was read from (e.g. "ro").
+    /// An [`ExecutionResult`].
     ///
     /// # Errors
     ///
@@ -363,10 +375,10 @@ impl<'execution> Executable<'_, 'execution> {
     ///
     /// # Returns
     ///
-    /// A `HashMap<String, ExecutionResult>` where the key is the name of the register that was read from (e.g. "ro").
+    /// An [`ExecutionResult`].
     ///
     /// # Errors
-    /// All errors are human readable by way of [`mod@eyre`]. Some common errors are:
+    /// All errors are human readable by way of [`mod@thiserror`]. Some common errors are:
     ///
     /// 1. You are not authenticated for QCS
     /// 1. Your credentials don't have an active reservation for the QPU you requested
@@ -487,7 +499,7 @@ pub enum Error {
     /// [`Executable::retrieve_results`] can invalidate the handle.
     #[error("The job handle was not valid")]
     InvalidJobHandle,
-    /// Occurs when failing to construct a [`QcsClient`].
+    /// Occurs when failing to construct a [`Qcs`] client.
     #[error("The QCS client configuration failed to load")]
     QcsConfigLoadFailure(#[from] LoadError),
 }

crates/lib/src/execution_data.rs

@@ -8,7 +8,6 @@ use std::time::Duration;
 use ndarray::prelude::*;
 
 use qcs_api_client_grpc::models::controller::{readout_values, ReadoutValues};
-use quil_rs::instruction::MemoryReference;
 
 use crate::RegisterData;
 
@@ -40,7 +39,7 @@ pub enum ReadoutValue {
 /// for all memory indexes of a given shot.
 pub type RegisterMatrix = Array2<Option<ReadoutValue>>;
 
-/// A mapping of readout fields to their [`ReadoutValues`].
+/// A mapping of readout fields to their [`ReadoutValue`]s.
 #[derive(Debug, Clone, PartialEq)]
 #[repr(transparent)]
 pub struct ReadoutMap(HashMap<String, RegisterMatrix>);
@@ -109,7 +108,7 @@ impl ReadoutMap {
     #[must_use]
     pub fn get_shot_wise_matrix(&self, register_name: &str) -> Option<RegisterMatrix> {
         let register = self.0.get(register_name);
-        register.map(|matrix| matrix.clone().reversed_axes())
+        register.cloned().map(ArrayBase::reversed_axes)
     }
 
     /// `readout_values` maps program-defined readout to result-defined readout, e.g.:
@@ -146,8 +145,7 @@ impl ReadoutMap {
                             .collect(),
                         None => Vec::new(),
                     };
-                    // TODO handle possible truncation
-                    let shape = (reference.index as usize + 1, row.len());
+                    let shape = (reference.index + 1, row.len());
                     let matrix = result
                         .0
                         .entry(reference.name)
@@ -160,8 +158,7 @@ impl ReadoutMap {
                         })
                         .or_insert_with(|| Array2::from_elem(shape, None));
                     for (shot_num, value) in row.iter().enumerate() {
-                        // TODO handle possible truncation
-                        matrix[[reference.index as usize, shot_num]] = *value;
+                        matrix[[reference.index, shot_num]] = *value;
                     }
                 });
         }
@@ -228,6 +225,11 @@ impl ReadoutMap {
     }
 }
 
+struct MemoryReference {
+    name: String,
+    index: usize,
+}
+
 #[derive(Debug, thiserror::Error)]
 pub(crate) enum MemoryReferenceParseError {
     #[error("Could not parse memory reference: {reason}")]
@@ -256,7 +258,7 @@ fn parse_readout_register(
 
     Ok(MemoryReference {
         name: String::from(&register_name[..open_brace]),
-        index: u64::from_str(&register_name[open_brace + 1..close_brace])?,
+        index: usize::from_str(&register_name[open_brace + 1..close_brace])?,
     })
 }
 
@@ -265,6 +267,7 @@ mod describe_readout_map {
     use maplit::hashmap;
     use ndarray::prelude::*;
     use std::collections::HashMap;
+    use test_case::test_case;
 
     use super::{ReadoutMap, ReadoutValue, ReadoutValues, RegisterData};
     use qcs_api_client_grpc::models::controller::readout_values::Values;
@@ -343,4 +346,83 @@ mod describe_readout_map {
         ]);
         assert_eq!(ro, expected);
     }
+
+    #[test_case(0, 0 => Some(ReadoutValue::Integer(1)) ; "returns value when both indices are in bounds")]
+    #[test_case(1, 0 => None ; "returns None when row index is off by 1")]
+    #[test_case(0, 1 => None ; "returns None when column index is off by 1")]
+    fn get_value(row: usize, col: usize) -> Option<ReadoutValue> {
+        let registers: HashMap<Box<str>, RegisterData> = hashmap! {
+            String::from("ro").into() => RegisterData::I8(vec![vec![1]]),
+        };
+
+        ReadoutMap::from_register_data_map(&registers).get_value("ro", row, col)
+    }
+
+    #[test_case(0 => Some(vec![Some(ReadoutValue::Integer(1)), Some(ReadoutValue::Integer(2))]) ; "returns correct row when index is in bounds")]
+    #[test_case(2 => None ; "returns None when index is off by 1")]
+    fn get_values_by_memory_index(index: usize) -> Option<Vec<Option<ReadoutValue>>> {
+        let registers: HashMap<Box<str>, RegisterData> = hashmap! {
+            String::from("ro").into() => RegisterData::I8(vec![vec![1, 9], vec![2, 10]]),
+        };
+
+        ReadoutMap::from_register_data_map(&registers).get_values_by_memory_index("ro", index)
+    }
+
+    #[test_case(0 => Some(vec![Some(ReadoutValue::Integer(1)), Some(ReadoutValue::Integer(2))]) ; "returns correct column when index is in bounds")]
+    #[test_case(2 => None ; "returns None when index is off by 1")]
+    fn get_values_by_shot(shot_num: usize) -> Option<Vec<Option<ReadoutValue>>> {
+        let registers: HashMap<Box<str>, RegisterData> = hashmap! {
+            String::from("ro").into() => RegisterData::I8(vec![vec![1, 2], vec![3, 4]]),
+        };
+
+        ReadoutMap::from_register_data_map(&registers).get_values_by_shot("ro", shot_num)
+    }
+
+    #[test]
+    fn test_get_index_wise_matrix() {
+        let registers: HashMap<Box<str>, RegisterData> = hashmap! {
+            String::from("ro").into() => RegisterData::I8(vec![vec![1, 3], vec![2, 4]]),
+        };
+
+        let expected = arr2(&[
+            [
+                Some(ReadoutValue::Integer(1)),
+                Some(ReadoutValue::Integer(2)),
+            ],
+            [
+                Some(ReadoutValue::Integer(3)),
+                Some(ReadoutValue::Integer(4)),
+            ],
+        ]);
+
+        let readout_data = ReadoutMap::from_register_data_map(&registers);
+        let matrix = readout_data
+            .get_index_wise_matrix("ro")
+            .expect("ReadoutMap should have ro");
+        assert_eq!(matrix, expected);
+    }
+
+    #[test]
+    fn test_get_shot_wise_matrix() {
+        let registers: HashMap<Box<str>, RegisterData> = hashmap! {
+            String::from("ro").into() => RegisterData::I8(vec![vec![1, 3], vec![2, 4]]),
+        };
+
+        let expected = arr2(&[
+            [
+                Some(ReadoutValue::Integer(1)),
+                Some(ReadoutValue::Integer(3)),
+            ],
+            [
+                Some(ReadoutValue::Integer(2)),
+                Some(ReadoutValue::Integer(4)),
+            ],
+        ]);
+
+        let readout_data = ReadoutMap::from_register_data_map(&registers);
+        let matrix = readout_data
+            .get_shot_wise_matrix("ro")
+            .expect("ReadoutMap should have ro");
+        assert_eq!(matrix, expected);
+    }
 }

crates/lib/src/qpu/client.rs

@@ -193,7 +193,7 @@ pub enum GrpcClientError {
     GrpcError(#[from] GrpcError),
 }
 
-/// Errors that may occur while trying to use a [`OpenAPI`] client
+/// Errors that may occur while trying to use an ``OpenAPI`` client
 #[derive(Debug, thiserror::Error)]
 pub enum OpenApiClientError<T> {
     /// Error due to request failure

crates/lib/src/qpu/quilc/mod.rs

@@ -170,7 +170,7 @@ impl NativeQuilRequest {
     }
 }
 
-/// Description of a device to compile for, part of [`NativeQuilRequest`]
+/// Description of a device to compile for.
 #[derive(Serialize, Deserialize, Debug, Clone, PartialEq)]
 #[serde(tag = "_type")]
 pub struct TargetDevice {