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.

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

Add builtin interner index for empty qubits or clbits #12932

Closed
mtreinish opened this issue Aug 9, 2024 · 2 comments
Closed

Add builtin interner index for empty qubits or clbits #12932

mtreinish opened this issue Aug 9, 2024 · 2 comments
Labels
Rust This PR or issue is related to Rust code in the repository type: enhancement It's working, but needs polishing

Comments

@mtreinish
Copy link
Member

I have a change to the interners I want to make once the DAGCircuit PR merges that will avoid the need for the Vec here in favour of the static empty slice (which is at least a small saving), but I think the idea of the special "intern key to the 'zero' object" is a good one too. I can see it coming up a lot, and I think it's doable without losing any particular generality in the interners - we can make it like

pub struct Interner<T> { ... };
pub struct InternKey<T> {
   index: u32,
   _phantom: PhantomData<T>,
};
impl<T: Default> InternKey<T> {
    pub fn empty() -> Self {
        Self {
            val: 0,
            _phantom: PhantomData,
        }
    }
}

or something like that, and arrange that the 0 index of Interner<T: Default> always corresponds to <T as Default>::default().

Originally posted by @jakelishman in #12809 (comment)

@mtreinish mtreinish added Rust This PR or issue is related to Rust code in the repository enhancement labels Aug 9, 2024
@jakelishman
Copy link
Member

I thought about this more overnight, and I think that while specialisation isn't a stable part of Rust, it's hard to make the exact API I sketched above work exactly right, unless we require T: Default for all Interner generics - the trick is that Interner::<T>::new needs to always enforce that the default entry is always created (if T: Default), and we need specialisation for that to work natively.

I think overall the general scheme works, just the API I roughly sketched in the comment isn't 100% there yet.

@1ucian0 1ucian0 added type: enhancement It's working, but needs polishing and removed enhancement labels Sep 2, 2024
@jakelishman
Copy link
Member

This was done in #13035.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Rust This PR or issue is related to Rust code in the repository type: enhancement It's working, but needs polishing
Projects
None yet
Development

No branches or pull requests

3 participants