Move Bit and Register to live in Rust space
#13860
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Co-authored-by: Jake Lishman <[email protected]>
- Use `macro_rules!` to create `Bit` and `Registers`.
- Add `new_owned` method for `BitInfo`.
Pull Request Test Coverage Report for Build 13547192049Warning: This coverage report may be inaccurate.This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.
Details
💛 - Coveralls |
- Rebalance currently available api to allow for superclasses `Bit` and `Register` to also live in Rust. - Remove: `ShareableBit` triat and structs that implement it. Replace them with new structs.
- Add prefix class attribute for python `Register` instances.
- Add `BitExtraInfo` as a soft identifier for `BitInfo`, can be null to identify a `Bit`. - Add `RegisterInfo::get` method to retrieve the information of a `Bit`. - Have the rust registers own their counters instead of having them be Python exclusive. - Make subclassing of `Regster` and `Bit` a bit more effective by helping the subclasses inherit most methods.
raynelfss
force-pushed
the
bit-reg-rust
branch
from
15ee40d to
1d112c5
Compare
…GCircuit`. - Modify `BitData` to accept an extra generic value specifying a "sharable" object type that can also be turned into a `PyObject` to allow for compatibility with `Var`. - Rename `BitAsKey` to `VarAsKey` as it is currently only used for `Var` instances. - Modify methods in `CommutationChecker` and `GateDirection` passes to use the newer methods. - Other tweaks and fixes.
…bit`. - Remove imports of `QUBIT` and `CLBIT` from `DAGCircuit` and `CircuitData`.
- Discarded old equality and hashing methods for `PyBit` and `PyRegister`. - Fix `replace_bits` to accept any iterator over `Qubits` and `Clbits` from Python. - Add `is_empty` methods to Register data, wherever `len` is available. - Add additional parsing during creation of `Register` to more closely match previous behavior. - Modify `Bit` tests due to mocking no longer operating correctly.
- Add method `register` to `BitData` to better represent a register reference in an owned bit.
- Add missing quotation marks in `Register` repr().
- Bits and registers that live in Rust are not guaranteed to pass the `is` equality check in Python. - Restore type checking in `DAGCircuit` when adding bits.
- `Bits` should retain their hash value upon deserialization to avoid incorrect retrievals via hash or indexing. Fixed by implementing correct `__reduce__` method in `PyBit`, `PyClbit`, and `PyQubit`. - Replace `__getnnewargs__` methods with `__reduce__` for more versatile serialization. - Extend `SliceOrVec` to include a method that allows a vec with negative indices to correctly iterate based on a provided size. - Modify `FullAncillaAllocation` to not replace the `QuantumRegister` prefix. - Add `instances_count` getter attribute to all registers. - `is` comparisons are no longer guaranteed to work between bits and registers. So some tests have been modified to reflect that. - When `apply_operation` in the `DAGCircuit` receives a clbit in place of a qubit and viceversa, it will throw a type error, not a Key error. This is handled by PyO3 directly during extraction.
raynelfss
force-pushed
the
bit-reg-rust
branch
from
14e1541 to
25c5fbf
Compare
- Create `RegisterData` struct which, similarly to how `BitData` works, stores the registers and allows to access them by key, in this case by name or index. - Tweak `CircuitData` and `QuantumCircuit` api to allow for access of registers from within `CircuitData`. - Add `qubit_indices` and `clbit_indices` structs to keep track of the locations of bits and registers within the circuit. - Modify `circuit_to_dag` to obtain the registers directly from `CircuitData`. - Modify `BlueprintCircuit` to rely on `CircuitData` to access `QuantumRegister` instances. - Add setters and getters for registers. - Modify `circuit_to_instruction` to bring over the registers into the definition of the `Instruction`. - Add `contains` method to `BitData`. - Add rust native `BitLocations` that can be converted to a `Python` version if needed.
raynelfss
force-pushed
the
bit-reg-rust
branch
from
25c5fbf to
a578bf6
Compare
- Implemented `RegisterData` within the `DAGCircuit`. - Modified methods of the `DAGCircuit` to utilize new native structures wherever possible. - Modify `RegisterData` to not have a `description` attribute, and use an `IndexMap` to preserve insertion order of the registers. - Use native initializers for Registers throughout the crate. - Use native bits in initializer methods for `QuantumCircuit` and `ClassicalCircuit` instead of `BitData`. - Modify additional `is` checks from tests where not needed. - Modify `test_gate_definitions` to create owning registers when testing for definitions from the `EquivalenceLibrary`.
5 tasks
- Fix `remove` method to work more efficiently and add `remove_registers` for multiple removals. - Store index mapping between register name and `RegisterIndex<u32>` which is copyable.
- Now that the circuit contains all the available `Qubit` and `Clbit` information, this function can live entirely in Rust.
…Bits` and `Registers`. - Properly format code and skip certain buggy format checks.
raynelfss
added
priority: high
Rust
raynelfss
changed the title
Bit and Registers to live in Rust space
raynelfss
changed the title
Bit and Registers to live in Rust spaceBit and Register to live in Rust space
|
One or more of the following people are relevant to this code:
|
mtreinish
reviewed
Feb 25, 2025
Comment on lines
+19
to
+20
| other: | ||
| - | |
There was a problem hiding this comment.
This should be an upgrade note because it's an api change that users will need to potentially update their usage for on upgrading.
Suggested change
| other: | |
| - | | |
| - | |
Comment on lines
+12
to
+18
| :class:`.DAGCircuit` and :class:`.CircuitData` will now store :class:`.Qubit`, | ||
| :class:`.Clbit`, as well as its register counterparts, in Rust space. | ||
| - The :attr:`.QuantumCircuit._qubit_indices` and :attr:`.QuantumCircuit._clbit_indices` | ||
| attributes of the :class:`.QuantumCircuit` class have been moved to live in | ||
| rust space and store rust native data. | ||
| - The :meth:`.QuantumCircuit._qbit_argument_conversion` and :meth:`.QuantumCircuit._cbit_argument_conversion` | ||
| have been moved to Rust to operate with native data. |
There was a problem hiding this comment.
What are the user implications for this? An upgrade note is a potential call for action from end users saying you may need to change something when you upgrade. Just operating in rust space isn't really a user facing change. Looking at the test changes it's the note below about the is checks, maybe the TypeError instead of DAGCircuitError.
There was a problem hiding this comment.
Hmm, that's right, these functions weren't exposed to the public either way, so I could remove those parts.
Comment on lines
+4
to
+9
| Python classes :class:`.Bit` and :class:`.Register` have been ported to Rust | ||
| along with all their respective subclasses including: | ||
| * :class:`.Qubit` and :class:`.QuantumRegister`. | ||
| * :class:`.Clbit` and :class:`.ClassicalRegister`. | ||
| * :class:`.AncillaQubit` and :class:`.AncillaRegister`. |
There was a problem hiding this comment.
What's the user impact for the feature here? Is it faster, etc? The rust refactor is not really a user facing change because the interface the users consume hasn't changed. We normally reserve feature notes for user facing features.
There was a problem hiding this comment.
No user impact whatsoever, so then should I just remove that section and just mention the upgrade parts?
There was a problem hiding this comment.
Yeah, if there isn't a user impact I would just remove this from the release notes
- Add dispose method to `BitLocator` and `RegisterData`. - Make `BitInfo` private to the `circuit` crate. - Add `cached_raw` getter to `BitLocator`.
mtreinish
reviewed
Feb 26, 2025
| def __setstate__(self, state): | ||
| self._name, self._size, self._hash, self._repr, self._bits = state | ||
| self._bit_indices = None | ||
| Register = circuit.Register |
There was a problem hiding this comment.
Why not just do: from qiskit._accelerate.circuit import Register ?
There was a problem hiding this comment.
I was following what we do for importing CircuitInstruction, no reason other than that
| # Prefix to use for auto naming. | ||
| prefix = "a" | ||
| bit_type = AncillaQubit | ||
| Qubit = circuit.Qubit |
There was a problem hiding this comment.
This is kind of a weird pattern, normally you'd only do this if you needed to rename the object. You can just import the objects. If lint complains I would add an __all__ to say explicitly the module exports the classes.
Comment on lines
+1408
to
+1409
| qregs=_copy.deepcopy(self._data.qregs, memo), | ||
| cregs=_copy.deepcopy(self._data.cregs, memo), |
There was a problem hiding this comment.
Not for this PR necessarily, but I feel like this should all be handled by _data.copy(deepcopy=True) now. With everything living in rust now, it should be easier to just bake this into the CircuitData.copy() implementation (the whole replace_bits() call not just the register piece added here).
| @@ -87,7 +97,7 @@ def qregs(self, qregs): | |||
| return | |||
| self._qregs = [] | |||
| self._ancillas = [] | |||
| self._qubit_indices = {} | |||
| # self._qubit_indices = {} | |||
There was a problem hiding this comment.
Suggested change
| # self._qubit_indices = {} |
| @@ -39,7 +39,7 @@ def __init__(self, *regs, name: str | None = None) -> None: | |||
| super().__init__(*regs, name=name) | |||
| self._qregs: list[QuantumRegister] = [] | |||
| self._cregs: list[ClassicalRegister] = [] | |||
| self._qubit_indices = {} | |||
| # self._qubit_indices = {} | |||
There was a problem hiding this comment.
Suggested change
| # self._qubit_indices = {} |
Comment on lines
+238
to
+239
| items: PyList::type_object(py) | ||
| .call1((qreg.clone(),))? |
There was a problem hiding this comment.
Can't you just use the pyo3 constructor for a PyList here? Something like:
items: PyList::new(py, qreg.clone())?.unbind(),
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
The following commits finally bring
BitsandRegistersentirely to Rust by performing a couple of tasks:Bit(a.k.a.ShareableQubitandShareableClbit) andRegister(a.k.a.QuantumRegisterandClassicalRegister) and move their python counterparts to be managed by rust PyO3.DAGCircuitandCircuitData.RegisterDataandBitLocatorto represent the layout of bits/registers in the circuit.All these changes supersede #13686.
Details and comments
As we move more of our core data model to Rust, the main drawback that we've still had is needing to use Python space to create bits and registers. Although simple on the outside, the
BitandRegisterstructures in Python can be quite challenging due to cross referencing that can happen between each type to the other (Bit._register<->Register[i]] which is not as easy to replicate in Rust space.With the following commits we aim to move the representation of
BitsandRegistersto Rust in which the behavior is more similar to that of Python and allows us to share instances between circuits, rather than having the circuit have complete ownership of the bits it contains (which is the reason for preferring this over #13686).Additions
Bit representation:
To represent a
QubitorClbitin Rust, we have a couple of identifiers:QubitorClbitwhich is an index that the circuit can locally refer to when using a bit.Bit:ShareableQubitwhich represents a global bit.ShareableClbitthe classical counterpart to theShareableQubit.The base structure for a
Bitin Rust, calledBitInfodescribes a global bit as being one of two things:Ownedwhere the bit is owned by a register.Anonymouswhere the bit is independent, containing its own unique ID.Each of these
Bitsalso has an extra property calledBitExtraInfothat acts as an identifier, and contains any extra information aBitof a specific type should have. In the case ofShareableQubitwhether the instance is an ancilla or not.Python:
PyBit,PyQubit,PyClbitandPyAncillaQubit) have similar behavior and inheritance models as the originals, but should not be subclassed any further as it is no longer supported. See Deprecate subclassing ofRegisterandBit#13841.is()checks due to conversions in PyO3 not taking on the same addresses.IntoPyObject<'_>so that they can be successfully converted and extracted from their Python counterparts.Register representation:
To represent a
QuantumRegisterorClassicalRegisterin Rust, we can use the rust native counterparts of the same name:RegisterInfowhich classifies them into two categories:Owning: which owns its bits, can generate them as needed. All of its bits areOwned.Alias: A register that acts as a collection of bits independent from their type.RegisterDatastruct.Python:
RegisterandBit#13841. They may also not be compared using is(), and the native implementations can be converted to and from Python using the `IntoPyObject<'_> trait.CircuitData
CircuitData has been upgraded to contain registers, this is how it was achieved.
RegisterDatawas created just to save all of the registers stored in theCircuitDatainstance.PyDictto allow for a faster visit path from Python.BitLocatorit is essentially a wrapper around anIndexMapand emulates the behavior ofQuantumCircuit._{cl, qu}bit_indices.RegisterData, it also contains a cachedPyDictto enable faster access from python.{q,c}bit_argument_conversionsmethods inQuantumCircuitwhich perform mappings with native bit types.DAGCircuit
The DAGCircuit now uses
RegisterDataandBitLocatorin the same way they are used forCircuitData, getting rid of the manyPyDictobjects being stored within. Said additions removed the need of mostpytokens previously needed to modify the circuit's bits and registers.Questions and Comments
Feel free to leave a review/comment addressing any questions you may have. 🚀
Co-authored-by: Jake Lishman [email protected]