Skip to content

Commit

Permalink
Move interners from trait to generic structs
Browse files Browse the repository at this point in the history
This rewrites the interner mechanisms be defined in terms of two generic
structs (`Interner` and `Interned`) that formalise the idea that the
interned values are owned versions of a given reference type, and move
the logically separate intern-related actions (get a key from the value,
and get the value from the key) into separate functions (`get` and
`insert`, respectively).

This has a few advantages:

1. we now have both `insert` and `insert_owned` methods, which was
   awkward to add within the trait-based structure.  This allows a more
   efficient path when the owned variant has already necessarily been
   constructed.

2. additionally, the standard `insert` path now takes only a reference
   type.  For large circuits, most intern lookups will, in general, find
   a pre-existing key, so in situations where the interned value is
   sufficiently small that it can be within a static allocation (which
   is the case for almost all `qargs` and `cargs`), it's more efficient
   not to construct the owned type on the heap.

3. the type of the values retrieved from an interner are no longer
   indirected through the owned type that's stored.  For example,
   where `IndexedInterner<Vec<Qubit>>` gave out `&Vec<Qubit>`s as its
   lookups, `Interner<[Qubit]>` returns the more standard `&[Qubit]`,
   which is only singly indirect rather than double.

The following replacements are made:

1. The `IndexedInterner` struct from before is now just called
   `Interner` (and internally, it uses an `IndexSet` rather than
   manually tracking the indices).  Its generic type is related to the
   references it returns, rather than the owned value it stores, so
   `IndexedInterner<Vec<Qubit>>` becomes `Interner<[Qubit]>`.

2. The `Interner` trait is now gone.  Everything is just accessed as
   concrete methods on the `Interner` struct.

3. `<&IndexedInterner as Interner>::intern` (lookup of the value from an
   interner key) is now called `Interner::get`.

4. `<&mut IndexedInterner as Interner>::intern` (conversion of an owned
   value to an interner key) is now called `Interner::insert_owned`.

5. A new method, `Interner::insert`, can now be used when one need not
   have an owned allocation of the storage type; the correct value will
   be allocated if required (which is expected to be less frequent).

6. The intern key is no longer called `interner::Index`, but
   `Interned<T>`, where the generic parameter `T` matches the generic of
   the `Interner<T>` that gave out the key.
  • Loading branch information
jakelishman committed Aug 25, 2024
1 parent d3040a0 commit a34578e
Show file tree
Hide file tree
Showing 4 changed files with 220 additions and 164 deletions.
56 changes: 27 additions & 29 deletions crates/circuit/src/circuit_data.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ use std::cell::OnceCell;
use crate::bit_data::BitData;
use crate::circuit_instruction::{CircuitInstruction, OperationFromPython};
use crate::imports::{ANNOTATED_OPERATION, CLBIT, QUANTUM_CIRCUIT, QUBIT};
use crate::interner::{IndexedInterner, Interner};
use crate::interner::Interner;
use crate::operations::{Operation, OperationRef, Param, StandardGate};
use crate::packed_instruction::{PackedInstruction, PackedOperation};
use crate::parameter_table::{ParameterTable, ParameterTableError, ParameterUse, ParameterUuid};
Expand Down Expand Up @@ -91,9 +91,9 @@ pub struct CircuitData {
/// The packed instruction listing.
data: Vec<PackedInstruction>,
/// The cache used to intern instruction bits.
qargs_interner: IndexedInterner<Vec<Qubit>>,
qargs_interner: Interner<[Qubit]>,
/// The cache used to intern instruction bits.
cargs_interner: IndexedInterner<Vec<Clbit>>,
cargs_interner: Interner<[Clbit]>,
/// Qubits registered in the circuit.
qubits: BitData<Qubit>,
/// Clbits registered in the circuit.
Expand Down Expand Up @@ -148,8 +148,8 @@ impl CircuitData {
global_phase,
)?;
for (operation, params, qargs, cargs) in instruction_iter {
let qubits = (&mut res.qargs_interner).intern(qargs)?;
let clbits = (&mut res.cargs_interner).intern(cargs)?;
let qubits = res.qargs_interner.insert_owned(qargs);
let clbits = res.cargs_interner.insert_owned(cargs);
let params = (!params.is_empty()).then(|| Box::new(params));
res.data.push(PackedInstruction {
op: operation,
Expand Down Expand Up @@ -199,9 +199,9 @@ impl CircuitData {
instruction_iter.size_hint().0,
global_phase,
)?;
let no_clbit_index = (&mut res.cargs_interner).intern(Vec::new())?;
let no_clbit_index = res.cargs_interner.insert(&[]);
for (operation, params, qargs) in instruction_iter {
let qubits = (&mut res.qargs_interner).intern(qargs.to_vec())?;
let qubits = res.qargs_interner.insert(&qargs);
let params = (!params.is_empty()).then(|| Box::new(params));
res.data.push(PackedInstruction {
op: operation.into(),
Expand All @@ -227,8 +227,8 @@ impl CircuitData {
) -> PyResult<Self> {
let mut res = CircuitData {
data: Vec::with_capacity(instruction_capacity),
qargs_interner: IndexedInterner::new(),
cargs_interner: IndexedInterner::new(),
qargs_interner: Interner::new(),
cargs_interner: Interner::new(),
qubits: BitData::new(py, "qubits".to_string()),
clbits: BitData::new(py, "clbits".to_string()),
param_table: ParameterTable::new(),
Expand Down Expand Up @@ -258,9 +258,9 @@ impl CircuitData {
params: &[Param],
qargs: &[Qubit],
) -> PyResult<()> {
let no_clbit_index = (&mut self.cargs_interner).intern(Vec::new())?;
let no_clbit_index = self.cargs_interner.insert(&[]);
let params = (!params.is_empty()).then(|| Box::new(params.iter().cloned().collect()));
let qubits = (&mut self.qargs_interner).intern(qargs.to_vec())?;
let qubits = self.qargs_interner.insert(qargs);
self.data.push(PackedInstruction {
op: operation.into(),
qubits,
Expand Down Expand Up @@ -351,8 +351,8 @@ impl CircuitData {
) -> PyResult<Self> {
let mut self_ = CircuitData {
data: Vec::new(),
qargs_interner: IndexedInterner::new(),
cargs_interner: IndexedInterner::new(),
qargs_interner: Interner::new(),
cargs_interner: Interner::new(),
qubits: BitData::new(py, "qubits".to_string()),
clbits: BitData::new(py, "clbits".to_string()),
param_table: ParameterTable::new(),
Expand Down Expand Up @@ -572,10 +572,10 @@ impl CircuitData {
let qubits = PySet::empty_bound(py)?;
let clbits = PySet::empty_bound(py)?;
for inst in self.data.iter() {
for b in self.qargs_interner.intern(inst.qubits) {
for b in self.qargs_interner.get(inst.qubits) {
qubits.add(self.qubits.get(*b).unwrap().clone_ref(py))?;
}
for b in self.cargs_interner.intern(inst.clbits) {
for b in self.cargs_interner.get(inst.clbits) {
clbits.add(self.clbits.get(*b).unwrap().clone_ref(py))?;
}
}
Expand Down Expand Up @@ -737,8 +737,8 @@ impl CircuitData {
// Get a single item, assuming the index is validated as in bounds.
let get_single = |index: usize| {
let inst = &self.data[index];
let qubits = self.qargs_interner.intern(inst.qubits);
let clbits = self.cargs_interner.intern(inst.clbits);
let qubits = self.qargs_interner.get(inst.qubits);
let clbits = self.cargs_interner.get(inst.clbits);
CircuitInstruction {
operation: inst.op.clone(),
qubits: PyTuple::new_bound(py, self.qubits.map_indices(qubits)).unbind(),
Expand Down Expand Up @@ -894,7 +894,7 @@ impl CircuitData {
for inst in other.data.iter() {
let qubits = other
.qargs_interner
.intern(inst.qubits)
.get(inst.qubits)
.iter()
.map(|b| {
Ok(self
Expand All @@ -905,7 +905,7 @@ impl CircuitData {
.collect::<PyResult<Vec<Qubit>>>()?;
let clbits = other
.cargs_interner
.intern(inst.clbits)
.get(inst.clbits)
.iter()
.map(|b| {
Ok(self
Expand All @@ -915,8 +915,8 @@ impl CircuitData {
})
.collect::<PyResult<Vec<Clbit>>>()?;
let new_index = self.data.len();
let qubits_id = Interner::intern(&mut self.qargs_interner, qubits)?;
let clbits_id = Interner::intern(&mut self.cargs_interner, clbits)?;
let qubits_id = self.qargs_interner.insert_owned(qubits);
let clbits_id = self.cargs_interner.insert_owned(clbits);
self.data.push(PackedInstruction {
op: inst.op.clone(),
qubits: qubits_id,
Expand Down Expand Up @@ -1113,14 +1113,12 @@ impl CircuitData {
}

fn pack(&mut self, py: Python, inst: &CircuitInstruction) -> PyResult<PackedInstruction> {
let qubits = Interner::intern(
&mut self.qargs_interner,
self.qubits.map_bits(inst.qubits.bind(py))?.collect(),
)?;
let clbits = Interner::intern(
&mut self.cargs_interner,
self.clbits.map_bits(inst.clbits.bind(py))?.collect(),
)?;
let qubits = self
.qargs_interner
.insert_owned(self.qubits.map_bits(inst.qubits.bind(py))?.collect());
let clbits = self
.cargs_interner
.insert_owned(self.clbits.map_bits(inst.clbits.bind(py))?.collect());
Ok(PackedInstruction {
op: inst.operation.clone(),
qubits,
Expand Down
Loading

0 comments on commit a34578e

Please sign in to comment.