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

Jump to bottom

Move interners from trait to generic structs #13033

Merged
merged 3 commits into from
Aug 27, 2024
Merged

Move interners from trait to generic structs #13033

merged 3 commits into from
Aug 27, 2024

Commits on Aug 27, 2024

  1. Move interners from trait to generic structs 

    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.
    @jakelishman
    jakelishman committed Aug 27, 2024
    Configuration menu
    Copy the full SHA
    adfd16e View commit details
    Browse the repository at this point in the history

  2. Improve internal documentation

    @jakelishman
    jakelishman committed Aug 27, 2024
    Configuration menu
    Copy the full SHA
    2d56e17 View commit details
    Browse the repository at this point in the history

  3. Match names of interners between CircuitData and DAGCircuit

    @jakelishman
    jakelishman committed Aug 27, 2024
    Configuration menu
    Copy the full SHA
    fb71771 View commit details
    Browse the repository at this point in the history