Rebalance CircuitInstruction and PackedInstruction
#12730
Merged
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
This is a large overhaul of how circuit instructions are both stored in
Rust (`PackedInstruction`) and how they are presented to Python
(`CircuitInstruction`). In summary:
* The old `OperationType` enum is now collapsed into a manually managed
`PackedOperation`. This is logically equivalent, but stores a
`PyGate`/`PyInstruction`/`PyOperation` indirectly through a boxed
pointer, and stores a `StandardGate` inline. As we expect the vast
majority of gates to be standard, this hugely reduces the memory
usage. The enumeration is manually compressed to a single pointer,
hiding the discriminant in the low, alignment-required bytes of the
pointer.
* `PackedOperation::view()` unpacks the operation into a proper
reference-like enumeration `OperationRef<'a>`, which implements
`Operation` (though there is also a `try_standard_gate` method to get
the gate without unpacking the whole enumeration).
* Both `PackedInstruction` and `CircuitInstruction` use this
`PackedOperation` as the operation storage.
* `PackedInstruction` is now completely the Rust-space format for data,
and `CircuitInstruction` is purely for communication with Python.
On my machine, this commit brings the utility-scale benchmarks to within
10% of the runtime of 1.1.0 (and some to parity), despite all the
additional overhead.
Changes to accepting and building Python objects
------------------------------------------------
* A `PackedInstruction` is created by copy constructor from a
`CircuitInstruction` by `CircuitData::pack`. There is no `pack_owned`
(really, there never was - the previous method didn't take ownership)
because there's never owned `CircuitInstruction`s coming in; they're
Python-space interop, so we never own them (unless we clone them)
other than when we're unpacking them.
* `PackedInstruction` is currently just created manually when not coming
from a `CircuitInstruction`. It's not hard, and makes it easier to
re-use known intern indices than to waste time re-interning them.
There is no need to go via `CircuitInstruction`.
* `CircuitInstruction` now has two separated Python-space constructors:
the old one, which is the default and takes `(operation, qubits,
clbits)` (and extracts the information), and a new fast-path
`from_standard` which asks only for the standard gate, qubits and
params, avoiding operator construction.
* To accept a Python-space operation, extract a Python object to
`OperationFromPython`. This extracts the components that are separate
in Rust space, but joined in Python space (the operation, params and
extra attributes). This replaces `OperationInput` and
`OperationTypeConstruct`, being more efficient at the extraction,
including providing the data in the formats needed for
`PackedInstruction` or `CircuitInstruction`.
* To retrieve the Python-space operation, use
`CircuitInstruction::get_operation` or
`PackedInstruction::unpack_py_op` as appropriate. Both will
cache and reuse the op, if `cache_pygates` is active. (Though note
that if the op is created by `CircuitInstruction`, it will not
propagate back to a `PackedInstruction`.)
Avoiding operation creation
---------------------------
The `_raw_op` field of `CircuitInstruction` is gone, because `PyGate`,
`PyInstruction` and `PyOperation` are no longer pyclasses and no longer
exposed to Python. Instead, we avoid operation creation by:
* having an internal `DAGNode::_to_circuit_instruction`, which returns a
copy of the internal `CircuitInstruction`, which can then be used with
`CircuitInstruction.replace`, etc.
* having `CircuitInstruction::is_standard_gate` to query from Python
space if we should bother to create the operator.
* changing `CircuitData::map_ops` to `map_nonstandard_ops`, and having
it only call the Python callback function if the operation is not an
unconditional standard gate.
Memory usage
------------
Given the very simple example construction script:
```python
from qiskit.circuit import QuantumCircuit
qc = QuantumCircuit(1_000)
for _ in range(3_000):
for q in qc.qubits:
qc.rz(0.0, q)
for q in qc.qubits:
qc.rx(0.0, q)
for q in qc.qubits:
qc.rz(0.0, q)
for a, b in zip(qc.qubits[:-1], qc.qubits[1:]):
qc.cx(a, b)
```
This uses 1.5GB in max resident set size on my Macbook (note that it's
about 12 million gates) on both 1.1.0 and with this commit, so we've
undone our memory losses. The parent of this commit uses 2GB.
However, we're in a strong position to beat 1.1.0 in the future now;
there are two obvious large remaining costs:
* There are 16 bytes per `PackedInstruction` for the Python-operation
caching (worth about 180MB in this benchmark, since no Python
operations are actually created).
* There is also significant memory wastage in the current
`SmallVec<[Param; 3]>` storage of the parameters; for all standard
gates, we know statically how many parameters are / should be stored,
and we never need to increase the capacity. Further, the `Param` enum
is 16 bytes wide per parameter, of which nearly 8 bytes is padding,
but for all our current use cases, we only care if _all_ the
parameters or floats (for everything else, we're going to have to
defer to Python). We could move the discriminant out to the level of
the parameters structure, and save a large amount of padding.
Further work
------------
There's still performance left on the table here:
* We still copy-in and copy-out of `CircuitInstruction` too much right
now; we might want to make all the `CircuitInstruction` fields
nullable and have `CircuitData::append` take them by _move_ rather
than by copy.
* The qubits/clbits interner requires owned arrays going in, but most
interning should return an existing entry. We probably want to switch
to have the interner take references/iterators by default, and clone
when necessary. We could have a small circuit optimisation where the
intern contexts reserve the first n entries to use for an all-to-all
connectivity interning for up to (say) 8 qubits, since the transpiler
will want to create a lot of ephemeral small circuits.
* The `Param` vectors are too heavy at the moment; `SmallVec<[Param;
3]>` is 56 bytes wide, despite the vast majority of gates we care
about having at most one single float (8 bytes). Dead padding is a
large chunk of the memory use currently.
jakelishman
added
priority: high
performance
Changelog: None
jakelishman
requested review from
alexanderivrii,
ShellyGarion and
a team
as code owners
|
One or more of the following people are relevant to this code:
|
|
Benchmarking against its parent commit (bb60891), showing only changed benchmarks. This is almost universally better across the board of circuit construction and manipulation, by half to a full order of magnitude. Transpile and DAG-related operations are faster too, most likely because the conversions are slightly cheaper now and there's less data to iterate through on copies. |
|
Benchmarking against 1.1.0, showing only changed. Note that most utility-scale transpilations are not in this list - they're within 10% of 1.1.0 now. |
Pull Request Test Coverage Report for Build 10045737675Details
💛 - Coveralls |
|
I was a bit concerned by these numbers:
since failed either indicates it is no longer running or more likely in this case we hit the 60sec timeout. So to validate I ran the utility benchmarks locally with this PR compared against 1.1.0: So that failed result might be something tied to @jakelishman's local environment. Like it could be are old friend the medium allocator on MacOS with x86_64. |
3 tasks
kevinhartman
previously approved these changes
Jul 8, 2024
There was a problem hiding this comment.
This looks excellent! It's nice to see your idea of using the low 3 bits of 8-byte aligned pointers to store the operation type enum come to fruition 😄. I'm also glad to see some cleanup of our Rust operation types and conversion to and from Python. The performance and memory improvements are also nice, ofc.
I think the impact this has on our work to port DAGCircuit to Rust should be fairly minimal.
I'm approving this, but holding off on queuing it for merge in case anyone else wants to take a look.
|
I'd like look at it briefly @kevinhartman . |
This was a highly nontrivial merge, since bringing in more control-flow operations and allowing them to be cast down to standard gates found some problems in the model (both pre-existing and new for this PR).
|
I've fixed the merge conflicts with #12659 in c4f299f, the process of which turned up a couple more ugly corners around conditional / otherwise non-standard "standard" gates during the conversion. Notably: the use of Previously, overzealous rejection of standard gates in edit: ugh, in the mean time, #12704 merged which invalidates the merge. |
There was no actual merge conflict, but still a logical one.
This was referenced Jul 9, 2024
|
fwiw, I also got no failures running a few parts of the asv set including the two failures above. Using Arch linux with an AMD processor. |
|
It turns out that while asv's timeout is 60s, that includes all runs of a benchmark. Since we have a minimum of 2 runs for those the cut-off is actually 30s - I increased the timeout locally and it was running them in ~30.9s on my laptop or something. |
The `compose` test had a now-broken assumption, because the Python-space `is` check is no longer expected to return an identical object when a standard gate is moved from one circuit to another and has its components remapped as part of the `compose` operation. This doesn't constitute the unpleasant deep-copy that that test is preventing. A custom gate still satisfies that, however, so we can just change the test. `DAGNode::set_name` could cause problems if it was called for the first time on a `CircuitInstruction` that was for a standard gate; these would be created as immutable instances. Given the changes in operator extraction to Rust space, it can now be the case that a standard gate that comes in as mutable is unpacked into Rust space, the cache is some time later invalidated, and then the operation is recreated immutably.
raynelfss
added a commit
to raynelfss/qiskit
that referenced
this pull request
Jul 24, 2024
raynelfss
added a commit
to raynelfss/qiskit
that referenced
this pull request
Jul 24, 2024
6 tasks
raynelfss
added a commit
to raynelfss/qiskit
that referenced
this pull request
Jul 24, 2024
raynelfss
added a commit
to raynelfss/qiskit
that referenced
this pull request
Jul 24, 2024
ElePT
pushed a commit
to mtreinish/qiskit-core
that referenced
this pull request
Jul 24, 2024
* Rebalance `CircuitInstruction` and `PackedInstruction`
This is a large overhaul of how circuit instructions are both stored in
Rust (`PackedInstruction`) and how they are presented to Python
(`CircuitInstruction`). In summary:
* The old `OperationType` enum is now collapsed into a manually managed
`PackedOperation`. This is logically equivalent, but stores a
`PyGate`/`PyInstruction`/`PyOperation` indirectly through a boxed
pointer, and stores a `StandardGate` inline. As we expect the vast
majority of gates to be standard, this hugely reduces the memory
usage. The enumeration is manually compressed to a single pointer,
hiding the discriminant in the low, alignment-required bytes of the
pointer.
* `PackedOperation::view()` unpacks the operation into a proper
reference-like enumeration `OperationRef<'a>`, which implements
`Operation` (though there is also a `try_standard_gate` method to get
the gate without unpacking the whole enumeration).
* Both `PackedInstruction` and `CircuitInstruction` use this
`PackedOperation` as the operation storage.
* `PackedInstruction` is now completely the Rust-space format for data,
and `CircuitInstruction` is purely for communication with Python.
On my machine, this commit brings the utility-scale benchmarks to within
10% of the runtime of 1.1.0 (and some to parity), despite all the
additional overhead.
Changes to accepting and building Python objects
------------------------------------------------
* A `PackedInstruction` is created by copy constructor from a
`CircuitInstruction` by `CircuitData::pack`. There is no `pack_owned`
(really, there never was - the previous method didn't take ownership)
because there's never owned `CircuitInstruction`s coming in; they're
Python-space interop, so we never own them (unless we clone them)
other than when we're unpacking them.
* `PackedInstruction` is currently just created manually when not coming
from a `CircuitInstruction`. It's not hard, and makes it easier to
re-use known intern indices than to waste time re-interning them.
There is no need to go via `CircuitInstruction`.
* `CircuitInstruction` now has two separated Python-space constructors:
the old one, which is the default and takes `(operation, qubits,
clbits)` (and extracts the information), and a new fast-path
`from_standard` which asks only for the standard gate, qubits and
params, avoiding operator construction.
* To accept a Python-space operation, extract a Python object to
`OperationFromPython`. This extracts the components that are separate
in Rust space, but joined in Python space (the operation, params and
extra attributes). This replaces `OperationInput` and
`OperationTypeConstruct`, being more efficient at the extraction,
including providing the data in the formats needed for
`PackedInstruction` or `CircuitInstruction`.
* To retrieve the Python-space operation, use
`CircuitInstruction::get_operation` or
`PackedInstruction::unpack_py_op` as appropriate. Both will
cache and reuse the op, if `cache_pygates` is active. (Though note
that if the op is created by `CircuitInstruction`, it will not
propagate back to a `PackedInstruction`.)
Avoiding operation creation
---------------------------
The `_raw_op` field of `CircuitInstruction` is gone, because `PyGate`,
`PyInstruction` and `PyOperation` are no longer pyclasses and no longer
exposed to Python. Instead, we avoid operation creation by:
* having an internal `DAGNode::_to_circuit_instruction`, which returns a
copy of the internal `CircuitInstruction`, which can then be used with
`CircuitInstruction.replace`, etc.
* having `CircuitInstruction::is_standard_gate` to query from Python
space if we should bother to create the operator.
* changing `CircuitData::map_ops` to `map_nonstandard_ops`, and having
it only call the Python callback function if the operation is not an
unconditional standard gate.
Memory usage
------------
Given the very simple example construction script:
```python
from qiskit.circuit import QuantumCircuit
qc = QuantumCircuit(1_000)
for _ in range(3_000):
for q in qc.qubits:
qc.rz(0.0, q)
for q in qc.qubits:
qc.rx(0.0, q)
for q in qc.qubits:
qc.rz(0.0, q)
for a, b in zip(qc.qubits[:-1], qc.qubits[1:]):
qc.cx(a, b)
```
This uses 1.5GB in max resident set size on my Macbook (note that it's
about 12 million gates) on both 1.1.0 and with this commit, so we've
undone our memory losses. The parent of this commit uses 2GB.
However, we're in a strong position to beat 1.1.0 in the future now;
there are two obvious large remaining costs:
* There are 16 bytes per `PackedInstruction` for the Python-operation
caching (worth about 180MB in this benchmark, since no Python
operations are actually created).
* There is also significant memory wastage in the current
`SmallVec<[Param; 3]>` storage of the parameters; for all standard
gates, we know statically how many parameters are / should be stored,
and we never need to increase the capacity. Further, the `Param` enum
is 16 bytes wide per parameter, of which nearly 8 bytes is padding,
but for all our current use cases, we only care if _all_ the
parameters or floats (for everything else, we're going to have to
defer to Python). We could move the discriminant out to the level of
the parameters structure, and save a large amount of padding.
Further work
------------
There's still performance left on the table here:
* We still copy-in and copy-out of `CircuitInstruction` too much right
now; we might want to make all the `CircuitInstruction` fields
nullable and have `CircuitData::append` take them by _move_ rather
than by copy.
* The qubits/clbits interner requires owned arrays going in, but most
interning should return an existing entry. We probably want to switch
to have the interner take references/iterators by default, and clone
when necessary. We could have a small circuit optimisation where the
intern contexts reserve the first n entries to use for an all-to-all
connectivity interning for up to (say) 8 qubits, since the transpiler
will want to create a lot of ephemeral small circuits.
* The `Param` vectors are too heavy at the moment; `SmallVec<[Param;
3]>` is 56 bytes wide, despite the vast majority of gates we care
about having at most one single float (8 bytes). Dead padding is a
large chunk of the memory use currently.
* Fix clippy in no-gate-cache mode
* Fix pylint unused-import complaints
* Fix broken assumptions around the gate model
The `compose` test had a now-broken assumption, because the Python-space
`is` check is no longer expected to return an identical object when a
standard gate is moved from one circuit to another and has its
components remapped as part of the `compose` operation. This doesn't
constitute the unpleasant deep-copy that that test is preventing. A
custom gate still satisfies that, however, so we can just change the
test.
`DAGNode::set_name` could cause problems if it was called for the first
time on a `CircuitInstruction` that was for a standard gate; these would
be created as immutable instances. Given the changes in operator
extraction to Rust space, it can now be the case that a standard gate
that comes in as mutable is unpacked into Rust space, the cache is some
time later invalidated, and then the operation is recreated immutably.
* Fix lint
* Fix minor documentation
raynelfss
added a commit
to raynelfss/qiskit
that referenced
this pull request
Jul 24, 2024
raynelfss
added a commit
to raynelfss/qiskit
that referenced
this pull request
Jul 25, 2024
raynelfss
added a commit
to raynelfss/qiskit
that referenced
this pull request
Jul 25, 2024
github-merge-queue bot
pushed a commit
that referenced
this pull request
Jul 25, 2024
…12292) * Initial: Add `Target` class to `_accelerate` - Add `Target` class to test mobility between Rust and Python. - Add `add_instruction` method to test compatibility with instructions. * Fix: Remove empty property check - Property check caused most cases to panic. - Will be commented out and restored at a later time. * Add: Instructions property - Instructions property returns all added to the target. - Similar behavior to source. * Chore: comments and deprecated methods - Add comments to instruction property. - Use new_bound for new PyDicts. * Chore: Remove redundant code - Remove redundant transformation of PyObject to PyTuple. - Remove debugging print statement. * Add: `InstructionProperties` class and type checkers - Add `InstructionProperties` class to process properties in rust. - Add `is_instance` and `is_class` to identify certain Python objects. - Modify logic of `add_instruction` to use class check. - Other tweaks and fixes. * Add: Setter and Getter for calibration in `InstructionProperty` * Add: `update_instruction_properties` to Target. * Add: Update_from_instruction_schedule_map - Partial addition from Target.py\ - Introduction of hashable qarg data structure. - Other tweaks and fixes. * Add: Complete `update_from_instruction_schedule_map1 - Complete missing procedures in function. - Rename `Qargs` to `HashableVec`. - Make `HashableVec` generic. - Separate `import_from_module_call` into call0 and call1. - Other tweaks and fixes. * Add: instruction_schedule_map property. - Remove stray print statements. - Other tweaks and fixes. * Fix: Key issue in `update_from_instruction_schedule_map` - Remove all unsafe unwraps * Fix: Use PyResult Value for void functon - Update `update_from_instruction_schedule_map to use PyResult and '?' operator. - Use Bound Python objects whenever possible. - Other tweaks and fixes. * Add: Python wrapping for Target - Add temporary _target module for testing. - Remove update_from_instruction_schedule_map function back to python. - Add python properties for all public attributes in rust - Other tweaks and fixes. * Add: `qargs` property - Add identical method `qargs` to obtain the qargs of a target. - Other tweaks and fixes. * Add: `qargs_for_operation_name` function. - Add function with identical behavior to the original in Target. - Other tweaks and fixes. * Add: durations method for Target - Add target module to qiskit init file. - Remove is_instance method. - Modify set_calibration method in InstructionProperty to leave typechecking to Python. - Change rust Target alias to Target2. - Other tweaks and fixes, * Add: InstructionProperties wrapper in python * Fix: InstructionProperties could not receive calibrations - Fix wrong setters/getters for calibration in InstructionProperty object in rust. * Add: more methods to Target in `target.rs` - Add FromPyObject trait to Hashable vec to receive Tuples and transform them directly into this type. - Add operations_for_qargs for Target class in Rust side and Python. - Fix return dict keys for `qargs_for_operation_name`. - Add `timing_constrains` and `operation_from_name` to Python side. - Other tweaks and fixes. * Fix: missing return value in `operations_for_args` - Fix wrong name for function operation_for_qargs. - Fix missing return value in the python side. - Other tweaks and fixes. * Fix: Bad compatibility with InstructionProperties - Make `InstructionProperties` "_calibration" attribute visible. - Removed attribute "calibration", treat as class property. - Other tweaks and fixes * Add: `operation_names_for_qargs` to Target - Port class method to rust and connect to Python wrapper. - Other tweaks and fixes. * Add: instruction_supported method to rust and python: - Other tweaks and fixes. * Add: changes to add_instruction function to increase functionality. - These changes break current functionality of other functions, butemulate intended behavior better. - Fixes coming soon. * Fix: Backwards compatibility with `add_instruction` - Fixed wrong additions to HashMaps in the rust side causing instructions to be missing. - Other tweaks and fixes. * Fix: Gate Map behavior didn't match #11422 - Make GateMap use optional values to match behavior of #11422. - Define GateMapType for complex type in self.gate_map. - Throw Python KeyError exceptions from the rust side in `update_instruction_properties` and other functions. - Modify logic in subsequent functions that use gate_map optional values. - Other tweaks and fixes. * Add: `has_calibration` method to Target * Add: `get_calibraton` method to Target * Add: `instruction_properties` method to Target * Add: `build_coupling_map` and helper methods - `build_coupling_map`will remain in Python for now, along with its helper functions. - Make `gate_name_map` visible to python. - Add `coupling_graph` attribute to Target in Rust. - Other tweaks and fixes. * Add: `get_non_global_operation_names` to Target. - Add attributes `non_global_strict_basis` and `non_global_basis` as Optional. - Other tweaks and fixes. * Add: Missing properties - Add properties: operations, operation_names, and physical_qubits. - Reorganize properties placement. - Other tweaks and fixes. * Add: `from_configuration` classmethod to Target. - Add method that mimics the behavior of the python method. - Change concurrent_measurements to 2d Vec instead of a Vec of sets. - Other tweaks and fixes. * Add: Magic methods to Rust and Python - Add docstring to __init__. - Add __iter__, __getitem__, __len__, __contains__, keys, values, and items methods to rust. - Add equivalen methods to python + the __str__ method. - Make description an optional attribute in rust. - Other tweaks and fixes. * Fix: Bugs when fetching qargs or operations - Fix qarg_for_operation_name logic to account for None and throw correct exceptions. - Stringify description before sending in case of numerical descriptors. - Fix qarg to account for None entry. - Other tweaks and fixes. * Chore: Prepare for Draft PR - Remove _target.py testing file. - Fix incorrect initialization of calibration in InstructionProperties. - Other tweaks and fixes. * Fix: target not being recognized as a module - Add target to the pyext crate. - Change placement of target import for alphabetical ordering. - Other tweaks and fixes. * Fix: Change HashMap to IndexMap - Change from f32 to f64 precision. - Other tweaks and fixes. * Fix: Move InstructionProperties fully to Rust - Move InstructionProperties to rust. - Modify gate_map to accept an InstructionProprties object instead of PyObjecy. - Change update_instruction_properties to use Option InstructionProprtyird. - Remove InstructionProperties from target.py - Other tweaks and fixes. * Fix: Make Target inherit from Rust - Make Target inherit from the rust-side Target by using subclass attribute, then extending its functionality using python. - Switch from __init__ to __new__ to adapt to the Target rust class. - Modify every trait that worked with `target._Target` to use `super()` or `self` instead. - Fix repr in InstructionProperties to not show `Some()` when a value exists. - Fix `__str__` method in `Target` to not display "None" if no description is given. - Assume `num_qubits` is the first argument when an integer is provided as a first argument and nothing else is provided for second (Target initializer). - Return a set in operation_names instead of a Vec. - Other tweaks and fixes. * Fix: Recognize None in `operation_for_qargs`. - Fix module labels for each class in target.rs. - Use py.is_instance instead of passing isinstance to `instruction_supported`. - Modify `operations_for_qargs` to accept optional values less aggressively. Allow it to find instructions with no qargs. (NoneType). - Other tweaks and fixes. * Fix: Make InstructionProperties subclassable. - Fix get_non_global_operation_names to accept optional values and fix search set to use sorted values. - Fix __repr__ method in InstructionProperties to add punctuation. - Fix typo in python durations method. - Modify test to overload __new__ method instead of just __init__ (Possible breaking change). -Other tweaks and fixes. * Fix: errors in `instruction_properties` and others: - Allow `instruction_properties` method to view optional properties. - Allow `operation_names_for_qargs` to select class instructions when None is passed as a qarg. - Modify __str__ method to display error and duration times as int if the value is 0. - Other tweaks and fixes. * Fix: call `isclass` from rust, instead of passing it from Python. * Fix: Move `update_from_instruction_schedule_map` to rust. * Fix: Move `durations` to rust. * Fix: Move `timing_constraints` to rust * Fix: Move operations_from_name fully to rust * Fix: `instruction_supported` method: - Rewrite the logic of instruction_supported due to previous errors in the method. - Move `check_obj_params` to Rust. - Other tweaks and fixes. * Fix: errors in `from_configuration` class method. - Fix some of the logic when retrieving gates from `name_mapping`. - Remove function arguments in favor of implementing counterpart functions in rust. - Add qubit_props_list_from_props function and return rust datatypes. - Fix wrong error handling procedures when retrieving attributes from backend_property. - Other tweaks and fixes. * Fix: Import `InstructionScheduleMap` directly instead of passing. - `instruction_schedule_map()` now imports the classtype directly from rust instead of needing it to be passed from python. - Remove unused imports in `target.py`. - Ignore unused arguments in `test_extra_props_str`. - Other tweaks and fixes. * Docs: Add docstrings to rust functions - Remove redundant redefinitions in python. - Fix text_signatures for some rust functions. - Added lint exceptions to some necessary imports and function arguments. - Other tweaks and fixes. * Add: Make `Target` and `InstructionProperties` pickleable. - Add `__getstate__` and `__setstate__` methods to make both rust subclasses pickleable. * Fix: Wrong calibration assignment in __setstate__ - Use set_calibration to set the correct calibration argument. - Fix wrong signature in get_non_global_operation_names. - Other tweaks and fixes. * Refactor: HashableVec is now Qarg - Use `PhysicalQubit` instead of u32 for qargs. - Use a `SmallVec` of size 4 instead of a dynamic Vec. - Default to using the `Hash()` method embedded in `SmallVec`. - Add a Default method to easily unwrap Qarg objects. - Other tweaks and fixes. * Add: `get` function to target. - Remove some redundant cloning in code. - Other small fixes. * Fix: Remove unnecessary Optional values in gate_map. - Update every gate_map call to use the new format. - Other small tweaks and fixes. * Refactor: `calibration` is for `InstructionProperties` - Use python `None` instead of option to store `calibration` in `InstructionProperties`. - Adapt code to these changes. - Remove redundant implementation of Hash in Qargs. - Other tweaks and fixes. * Fix: Temporary speedup for `gate_map` access - Added temporary speedups to access the gate_map by returning the values as PyObjects. - Convert qargs to rust tuples instead of initializing a `PyTuple`. - Store `InstructionProperties` as a python ref in gate_map. (Will be changed in future updates). - Other tweaks anf fixes. * Fix: Incorrect extractions for `InstructionProperties` - Fix incorrect conversion of `InstructionProperties` to `Py<InstructionProperties>` - Fix incorrect extraction of qargs in `update_from_instruction_schedule_map` * Fix: Hide all private attributes in `Target` - Hide all private attributes of the `Target` to prevent unecessary cloning. - Other small tweaks and fixes. * Add: New representation of gate_map using new pyclasses: - Make Qarg a sequence pyclass. - Make QargPropsMap the new representation of a GateMap value. - Adapt the code to new structure. - TODO: Add missing magic methods for sequence and mapping objects. - Other small tweaks and fixes. * Add: Use custom datatypes to return values to Python. - Add QargSet datatype to return a set of qargs. - Works as return type for `Target.qargs` - Object is has itertype of QargSetIter. - Rename QargPropMap to PropsMap - Use iterator type IterPropsMap - Other small tweaks and fixes. * Fix: Extend `InstructionProperties` to be subclassable using `__init__: - Made a subclass of `InstructionProperties` that can be extended using an `__init__`method. - Revert previous changes to `test_target.py`. - Other tweaks and fixes. * Refactor: Split target into its own module - Reestructure the files to improve readability of code. - `instruction_properties.rs` contaisn the `InstructionProperties` class. - `mod.rs` contains the `Target` class. - `qargs.rs` contains the Qargs struct to store quantum arguments. - `property_map` contains the Qarg: Property Mapping that will be stored in the gate_map. - Add missing methods to PropsMap: - Add `PropsMapKeys` object to store the qargs as a set. - Add methods to compare and access `PropsMapKey`. - Add QargsOrTuple enum in Qargs to parse Qargs instantly. * Fix: Rest of failing tests in Target - Modify the `InstructionProperties` python wrapper. - InstructionProperties was not assigning properties to rust side. - Make duration in `InstructionProperties` setable. - Add `__eq__` method for `PropMap` to compare with other dicts. - `PropMapKeys` can only be compared with a Set. - Remove `qargs_for_operation_name` from `target.py` - Other small tweaks and fixes. * Add: New GateMap Structure - GateMap is now its own mapping object. - Add `__setstate__` and `__getstate__` methods for `PropMap` and `GateMap`. - Other small tweaks and fixes. * Fix: Make new subclasses pickleable - Add module location to `PropsMap`, `GateMap`, and `Qargs`. - Added default method to PropMap. - Made default method part of class initializers. - Other smalls tweaks and fixes. * Fix: Remove redundant lookup in Target (#12373) * Format: `mod.rs` qubit_comparison to one line. * Add: `GateMapKeys` object in GateMap: - Use IndexSet as a base to preserve the insertion order. - Other tweaks and fixes. * Add: __sub__ method to GateMapKeys * Fix: Modify `GateMap` to store values in Python heap. - Fix `GateMap.__iter__` to use an IndexKeys iterator. - Other small tweaks and fixes. * Fix: Remove duplicate import of `IndexSet::into_iter` in `GateMap`. - Make `__iter__` use the keys() method in `GateMap`. * Fix:: Adapt to target changes (#12288) - Fix lint stray imports. * Fix: Incorrect creation of parameters in `update_from_instruction_schedule_map` - Add `tupelize` function to create tuples from non-downcastable items. - Fix creation of Parameters by iterating through members of tuple object and mapping them to parameters in `update_from_instruction_schedule_map`. - Add missing logic for creating a Target with/without `qubit_properties`. - Add tuple conversion of `Qargs` to store items in a dict in `BasisTranslator` and `UnitarySynthesis` passes. - Cast `PropsMap` object to dict when comparing in `test_fake_backends.py`. - Modify logic of helper functions that receive a bound object reference, a second `py` not required as an argument. - Add set operation methods to `GateMapKeys`. - Other tweaks and fixes. * Fix: More failing tests - Fix repeated erroneous calls to `add_instruction` in `update_from_instruction_schedule_map` - Add missing condition in `instruction_supported` - Use `IndexSet` instead of `HashSet` for `QargsSet`. - Other small tweaks and fixes. * Add: Macro rules for qargs and other sequences. - Create `QargSet` and `PropsMap` using the new macros. - Return a `TargetOpNames` ordered set to python in `operation_names`. - Remove the Python side `operation_names.` - Fix faulty docstring in `target.py`. - Other tweaks and fixes. * Docs: Add necessary docstrings to all new rust functions. - Remove duplicate Iterator in GateMap. - Other small tweaks and fixes. * Fix: Use `GILOneCell` and remove `Qargs` - Use `GILOneCell` to import python modules only once at initialization. - Remove the custom data structure `Qargs` to avoid conversion overhead. - `Qargs` does not use `PhysicalQubits`, `u32` is used instead. - Fix `__setstate__ `and `__getstate__` methods for `PropsMap`, `GateMap`, and `key_like_set_iterator` macro_rule. - Update code to use the new structures. - TODO: Fix broken tests. * Fix: Cast `Qargs` to `Tuple` in specific situations - Use tupleize to cast `Qargs` to `Tuple` in `instructions`. - Use downcast to extract string in `add_instruction`. - Other tweaks and fixes. * Add: Make `Target` Representable in Rust - Rename `InstructionProperties` as `BaseInstructionProperties`. - Remove `Calibration` from the rust space. - Restore `gate_map`, `coupling_map`, `instruction_schedule_map`, and `instruction_durations` to rust. - Remove all unnecessary data structures from rust space. - Other tweaks and fixes. * Refactor: Remove previour changes to unrelated files. * Add: rust native functions to target - Added rust native functionality to target such that a `py` would not be needed to use one. - Add Index trait to make `Target` subscriptable. - Other small tweaks and fixes. * Fix: Remove all unnecessary python method calls. - Remove uage of `inspect.isclass`. - Rename `Target` to `BaseTarget` in the rust side. - Rename `err.rs` to `errors.rs`. - Remove rust-native `add_inst` and `update_inst` as Target should not be modified from Rust. - Made `add_instruction` and `update_instruction_properties` private in `BaseTarget`. - Add missing `get` method in `Target`. - Other tweaks and fixes * Format: Fix lint * Fix: Wrong return value for `BaseTarget.qargs` * Add: Temporary Instruction representation in rust. - Add temporary instruction representation to avoid repeated extraction from python. * Add: Native representation of coupling graph * Fix: Wrong attribute extraction for `GateRep` * Remove: `CouplingGraph` rust native representation. - Move to different PR. * Format: Remove stray whitespace * Add: `get_non_global_op_names` as a rust native function * Fix: Use Ahash for Hashing - Use ahash for hashing when possible. - Rename `BaseTarget` to `Target` in rust only. - Rename `BaseInstructionProperties` to `InstructionProperties` in rust only. - Remove optional logic from `generate_non_global_op_names`. - Use dict for `__setstate__` and `__getstate__` in `Target`. - Reduced the docstring for `Target` and `InstructionProperties`. - Other small tweaks and fixes. * Format: new changes to `lib.rs` * Format: Adapt to new lint rules * Fix: Use new gates infrastructure (#12459) - Create custom enum to collect either a `NormalOperation` or a `VariableOperation` depending on what is needed. - Add a rust native `is_instruction_supported` method to check whether a Target supports a certain instruction. - Make conversion methods from `circuit_instruction.rs` public. - Add comparison methods for `Param` in `operations.rs` - Remove need for isclass method in rustwise `add_instruction` - Other tweaks and fixes. * Format: Fix rust formatting * Add: rust-native method to obtain Operstion objects. * Add: Comparison methods for `Param` * FIx: Add display methods for `Params` * Format: Fix lint test * Format: Wrong merge conflict solve * Fix: Improve rust methods to use iterators. - Adapt the Python methods to leverage the rust native improvements. - Use python native structures for the Python methods. * Format: Remove extra blankspace * Fix: Remove `text_signature`, use `signature` instead. * Fix: Rectify the behavior of `qargs` - Keep insertion order by inserting all qargs into a `PySet`. - Perform conversion to `PyTuple` at insertion time leveraging the iterator architecture. - Remove python side counterpart to avoid double iteration. - Make rust-native `qargs` return an iterator. * Fix: Corrections from Matthew's review - Use `format!` for repr method in `InstructionProperties` - Rename `Variable` variant of `TargetInstruction` to `Variadic`. - Remove `internal_name` attribute from `TargetOperation`. - Remove `VariableOperation` class. - Use `u32` for `granularity`, `pulse_alignment`, and `acquire_alignment`. - Use `Option` to store nullable `concurrent_measurements. - Use `&str` instead of `String` for most function arguments. - Use `swap_remove` to deallocate items from the provided `properties` map in `add_instruction`. - Avoid cloning instructions, use `to_object()` instead. - Avoid using `.to_owned()`, use `.clone()` instead. - Remove mention of `RandomState`, use `ahash::HashSet` instead. - Move parameter check to python in `instruction_supported`. - Avoid exposing private attributes, use the available ones instead. - Filter out `Varidadic` Instructions as they're not supported in rust. - Use peekable iterator to peak at the next qargs in `generate_non_global_op_names`. - Rename `qarg_set` to `deduplicated_qargs` in `generate_non_global_op_names`. - Return iterator instances instead of allocated `Vec`. - Add `python_compare` and `python_is_instance` to perform object comparison with objects that satisfy the `ToPyObject` trait. - Other small tweaks and fixes. * Implement a nullable dict-like structure for IndexMap (#2) * Initial: Implement a nullable dict-like structure for IndexMap * FIx: Erroneous item extraction from Python - Fix error that caused `None` values to be ignored from `None` keys. - Removed mutability from rust function argument in `add_instruction`. - Object is mutably referenced after option unwrapping. - Add missing header in `nullable_index_map.rs`. - Add Clone as a `K` and/or `V` constraint in some of the iterators. - Remove `IntoPy` constraint from `NullableIndexMap<K, V>`. - Add `ToPyObject` trait to `NullableIndexMap<K, V>`. * Fix: inplace modification of Python dict. - Perform `None` extraction from rust. - Revert changes to `Target.py` * Fix: Avoid double iteration by using filter_map. * Docs: Add inline comments. * Fix: More specific error message in `NullableIndexMap` * Fix: Use `Mapping` as the metaclass for `Target` - Minor corrections from Matthew's review. * Fix: Make `Target` crate-private. - Due to the private nature of `NullableIndexMap`, the `Target` has to be made crate private. - Add temporary`allow(dead_code)` flag for the unused `Target` and `NullableIndexMap` methods. - Fix docstring of `Target` struct. - Fix docstring of `add_instruction`. - Make several python-only operations public so they can be used with other `PyClass` instances as long as they own the gil. - Modify `py_instruction_supported` to accept bound objects. - Use rust-native functions for some of the instance properties. - Rewrite `instruction` to return parameters as slice. - `operation_names` returns an `ExactSizeIterator`. - All rust-native methods that return an `OperationType` object, will return a `NormalOperation` instance which includes the `OperationType` and the parameters. * Fix: Comments from Matthew's review - Mention duplication in docstring for rust Target. - Use f"{*:g}" to avoid printing the floating point for 0 in `Target`'s repr method. - Add note mentioning future unit-tests in rust. * Fix: Adapt to #12730
raynelfss
added a commit
to raynelfss/qiskit
that referenced
this pull request
Jul 25, 2024
Procatv
pushed a commit
to Procatv/qiskit-terra-catherines
that referenced
this pull request
Aug 1, 2024
* Rebalance `CircuitInstruction` and `PackedInstruction`
This is a large overhaul of how circuit instructions are both stored in
Rust (`PackedInstruction`) and how they are presented to Python
(`CircuitInstruction`). In summary:
* The old `OperationType` enum is now collapsed into a manually managed
`PackedOperation`. This is logically equivalent, but stores a
`PyGate`/`PyInstruction`/`PyOperation` indirectly through a boxed
pointer, and stores a `StandardGate` inline. As we expect the vast
majority of gates to be standard, this hugely reduces the memory
usage. The enumeration is manually compressed to a single pointer,
hiding the discriminant in the low, alignment-required bytes of the
pointer.
* `PackedOperation::view()` unpacks the operation into a proper
reference-like enumeration `OperationRef<'a>`, which implements
`Operation` (though there is also a `try_standard_gate` method to get
the gate without unpacking the whole enumeration).
* Both `PackedInstruction` and `CircuitInstruction` use this
`PackedOperation` as the operation storage.
* `PackedInstruction` is now completely the Rust-space format for data,
and `CircuitInstruction` is purely for communication with Python.
On my machine, this commit brings the utility-scale benchmarks to within
10% of the runtime of 1.1.0 (and some to parity), despite all the
additional overhead.
Changes to accepting and building Python objects
------------------------------------------------
* A `PackedInstruction` is created by copy constructor from a
`CircuitInstruction` by `CircuitData::pack`. There is no `pack_owned`
(really, there never was - the previous method didn't take ownership)
because there's never owned `CircuitInstruction`s coming in; they're
Python-space interop, so we never own them (unless we clone them)
other than when we're unpacking them.
* `PackedInstruction` is currently just created manually when not coming
from a `CircuitInstruction`. It's not hard, and makes it easier to
re-use known intern indices than to waste time re-interning them.
There is no need to go via `CircuitInstruction`.
* `CircuitInstruction` now has two separated Python-space constructors:
the old one, which is the default and takes `(operation, qubits,
clbits)` (and extracts the information), and a new fast-path
`from_standard` which asks only for the standard gate, qubits and
params, avoiding operator construction.
* To accept a Python-space operation, extract a Python object to
`OperationFromPython`. This extracts the components that are separate
in Rust space, but joined in Python space (the operation, params and
extra attributes). This replaces `OperationInput` and
`OperationTypeConstruct`, being more efficient at the extraction,
including providing the data in the formats needed for
`PackedInstruction` or `CircuitInstruction`.
* To retrieve the Python-space operation, use
`CircuitInstruction::get_operation` or
`PackedInstruction::unpack_py_op` as appropriate. Both will
cache and reuse the op, if `cache_pygates` is active. (Though note
that if the op is created by `CircuitInstruction`, it will not
propagate back to a `PackedInstruction`.)
Avoiding operation creation
---------------------------
The `_raw_op` field of `CircuitInstruction` is gone, because `PyGate`,
`PyInstruction` and `PyOperation` are no longer pyclasses and no longer
exposed to Python. Instead, we avoid operation creation by:
* having an internal `DAGNode::_to_circuit_instruction`, which returns a
copy of the internal `CircuitInstruction`, which can then be used with
`CircuitInstruction.replace`, etc.
* having `CircuitInstruction::is_standard_gate` to query from Python
space if we should bother to create the operator.
* changing `CircuitData::map_ops` to `map_nonstandard_ops`, and having
it only call the Python callback function if the operation is not an
unconditional standard gate.
Memory usage
------------
Given the very simple example construction script:
```python
from qiskit.circuit import QuantumCircuit
qc = QuantumCircuit(1_000)
for _ in range(3_000):
for q in qc.qubits:
qc.rz(0.0, q)
for q in qc.qubits:
qc.rx(0.0, q)
for q in qc.qubits:
qc.rz(0.0, q)
for a, b in zip(qc.qubits[:-1], qc.qubits[1:]):
qc.cx(a, b)
```
This uses 1.5GB in max resident set size on my Macbook (note that it's
about 12 million gates) on both 1.1.0 and with this commit, so we've
undone our memory losses. The parent of this commit uses 2GB.
However, we're in a strong position to beat 1.1.0 in the future now;
there are two obvious large remaining costs:
* There are 16 bytes per `PackedInstruction` for the Python-operation
caching (worth about 180MB in this benchmark, since no Python
operations are actually created).
* There is also significant memory wastage in the current
`SmallVec<[Param; 3]>` storage of the parameters; for all standard
gates, we know statically how many parameters are / should be stored,
and we never need to increase the capacity. Further, the `Param` enum
is 16 bytes wide per parameter, of which nearly 8 bytes is padding,
but for all our current use cases, we only care if _all_ the
parameters or floats (for everything else, we're going to have to
defer to Python). We could move the discriminant out to the level of
the parameters structure, and save a large amount of padding.
Further work
------------
There's still performance left on the table here:
* We still copy-in and copy-out of `CircuitInstruction` too much right
now; we might want to make all the `CircuitInstruction` fields
nullable and have `CircuitData::append` take them by _move_ rather
than by copy.
* The qubits/clbits interner requires owned arrays going in, but most
interning should return an existing entry. We probably want to switch
to have the interner take references/iterators by default, and clone
when necessary. We could have a small circuit optimisation where the
intern contexts reserve the first n entries to use for an all-to-all
connectivity interning for up to (say) 8 qubits, since the transpiler
will want to create a lot of ephemeral small circuits.
* The `Param` vectors are too heavy at the moment; `SmallVec<[Param;
3]>` is 56 bytes wide, despite the vast majority of gates we care
about having at most one single float (8 bytes). Dead padding is a
large chunk of the memory use currently.
* Fix clippy in no-gate-cache mode
* Fix pylint unused-import complaints
* Fix broken assumptions around the gate model
The `compose` test had a now-broken assumption, because the Python-space
`is` check is no longer expected to return an identical object when a
standard gate is moved from one circuit to another and has its
components remapped as part of the `compose` operation. This doesn't
constitute the unpleasant deep-copy that that test is preventing. A
custom gate still satisfies that, however, so we can just change the
test.
`DAGNode::set_name` could cause problems if it was called for the first
time on a `CircuitInstruction` that was for a standard gate; these would
be created as immutable instances. Given the changes in operator
extraction to Rust space, it can now be the case that a standard gate
that comes in as mutable is unpacked into Rust space, the cache is some
time later invalidated, and then the operation is recreated immutably.
* Fix lint
* Fix minor documentation
Procatv
pushed a commit
to Procatv/qiskit-terra-catherines
that referenced
this pull request
Aug 1, 2024
…iskit#12292) * Initial: Add `Target` class to `_accelerate` - Add `Target` class to test mobility between Rust and Python. - Add `add_instruction` method to test compatibility with instructions. * Fix: Remove empty property check - Property check caused most cases to panic. - Will be commented out and restored at a later time. * Add: Instructions property - Instructions property returns all added to the target. - Similar behavior to source. * Chore: comments and deprecated methods - Add comments to instruction property. - Use new_bound for new PyDicts. * Chore: Remove redundant code - Remove redundant transformation of PyObject to PyTuple. - Remove debugging print statement. * Add: `InstructionProperties` class and type checkers - Add `InstructionProperties` class to process properties in rust. - Add `is_instance` and `is_class` to identify certain Python objects. - Modify logic of `add_instruction` to use class check. - Other tweaks and fixes. * Add: Setter and Getter for calibration in `InstructionProperty` * Add: `update_instruction_properties` to Target. * Add: Update_from_instruction_schedule_map - Partial addition from Target.py\ - Introduction of hashable qarg data structure. - Other tweaks and fixes. * Add: Complete `update_from_instruction_schedule_map1 - Complete missing procedures in function. - Rename `Qargs` to `HashableVec`. - Make `HashableVec` generic. - Separate `import_from_module_call` into call0 and call1. - Other tweaks and fixes. * Add: instruction_schedule_map property. - Remove stray print statements. - Other tweaks and fixes. * Fix: Key issue in `update_from_instruction_schedule_map` - Remove all unsafe unwraps * Fix: Use PyResult Value for void functon - Update `update_from_instruction_schedule_map to use PyResult and '?' operator. - Use Bound Python objects whenever possible. - Other tweaks and fixes. * Add: Python wrapping for Target - Add temporary _target module for testing. - Remove update_from_instruction_schedule_map function back to python. - Add python properties for all public attributes in rust - Other tweaks and fixes. * Add: `qargs` property - Add identical method `qargs` to obtain the qargs of a target. - Other tweaks and fixes. * Add: `qargs_for_operation_name` function. - Add function with identical behavior to the original in Target. - Other tweaks and fixes. * Add: durations method for Target - Add target module to qiskit init file. - Remove is_instance method. - Modify set_calibration method in InstructionProperty to leave typechecking to Python. - Change rust Target alias to Target2. - Other tweaks and fixes, * Add: InstructionProperties wrapper in python * Fix: InstructionProperties could not receive calibrations - Fix wrong setters/getters for calibration in InstructionProperty object in rust. * Add: more methods to Target in `target.rs` - Add FromPyObject trait to Hashable vec to receive Tuples and transform them directly into this type. - Add operations_for_qargs for Target class in Rust side and Python. - Fix return dict keys for `qargs_for_operation_name`. - Add `timing_constrains` and `operation_from_name` to Python side. - Other tweaks and fixes. * Fix: missing return value in `operations_for_args` - Fix wrong name for function operation_for_qargs. - Fix missing return value in the python side. - Other tweaks and fixes. * Fix: Bad compatibility with InstructionProperties - Make `InstructionProperties` "_calibration" attribute visible. - Removed attribute "calibration", treat as class property. - Other tweaks and fixes * Add: `operation_names_for_qargs` to Target - Port class method to rust and connect to Python wrapper. - Other tweaks and fixes. * Add: instruction_supported method to rust and python: - Other tweaks and fixes. * Add: changes to add_instruction function to increase functionality. - These changes break current functionality of other functions, butemulate intended behavior better. - Fixes coming soon. * Fix: Backwards compatibility with `add_instruction` - Fixed wrong additions to HashMaps in the rust side causing instructions to be missing. - Other tweaks and fixes. * Fix: Gate Map behavior didn't match Qiskit#11422 - Make GateMap use optional values to match behavior of Qiskit#11422. - Define GateMapType for complex type in self.gate_map. - Throw Python KeyError exceptions from the rust side in `update_instruction_properties` and other functions. - Modify logic in subsequent functions that use gate_map optional values. - Other tweaks and fixes. * Add: `has_calibration` method to Target * Add: `get_calibraton` method to Target * Add: `instruction_properties` method to Target * Add: `build_coupling_map` and helper methods - `build_coupling_map`will remain in Python for now, along with its helper functions. - Make `gate_name_map` visible to python. - Add `coupling_graph` attribute to Target in Rust. - Other tweaks and fixes. * Add: `get_non_global_operation_names` to Target. - Add attributes `non_global_strict_basis` and `non_global_basis` as Optional. - Other tweaks and fixes. * Add: Missing properties - Add properties: operations, operation_names, and physical_qubits. - Reorganize properties placement. - Other tweaks and fixes. * Add: `from_configuration` classmethod to Target. - Add method that mimics the behavior of the python method. - Change concurrent_measurements to 2d Vec instead of a Vec of sets. - Other tweaks and fixes. * Add: Magic methods to Rust and Python - Add docstring to __init__. - Add __iter__, __getitem__, __len__, __contains__, keys, values, and items methods to rust. - Add equivalen methods to python + the __str__ method. - Make description an optional attribute in rust. - Other tweaks and fixes. * Fix: Bugs when fetching qargs or operations - Fix qarg_for_operation_name logic to account for None and throw correct exceptions. - Stringify description before sending in case of numerical descriptors. - Fix qarg to account for None entry. - Other tweaks and fixes. * Chore: Prepare for Draft PR - Remove _target.py testing file. - Fix incorrect initialization of calibration in InstructionProperties. - Other tweaks and fixes. * Fix: target not being recognized as a module - Add target to the pyext crate. - Change placement of target import for alphabetical ordering. - Other tweaks and fixes. * Fix: Change HashMap to IndexMap - Change from f32 to f64 precision. - Other tweaks and fixes. * Fix: Move InstructionProperties fully to Rust - Move InstructionProperties to rust. - Modify gate_map to accept an InstructionProprties object instead of PyObjecy. - Change update_instruction_properties to use Option InstructionProprtyird. - Remove InstructionProperties from target.py - Other tweaks and fixes. * Fix: Make Target inherit from Rust - Make Target inherit from the rust-side Target by using subclass attribute, then extending its functionality using python. - Switch from __init__ to __new__ to adapt to the Target rust class. - Modify every trait that worked with `target._Target` to use `super()` or `self` instead. - Fix repr in InstructionProperties to not show `Some()` when a value exists. - Fix `__str__` method in `Target` to not display "None" if no description is given. - Assume `num_qubits` is the first argument when an integer is provided as a first argument and nothing else is provided for second (Target initializer). - Return a set in operation_names instead of a Vec. - Other tweaks and fixes. * Fix: Recognize None in `operation_for_qargs`. - Fix module labels for each class in target.rs. - Use py.is_instance instead of passing isinstance to `instruction_supported`. - Modify `operations_for_qargs` to accept optional values less aggressively. Allow it to find instructions with no qargs. (NoneType). - Other tweaks and fixes. * Fix: Make InstructionProperties subclassable. - Fix get_non_global_operation_names to accept optional values and fix search set to use sorted values. - Fix __repr__ method in InstructionProperties to add punctuation. - Fix typo in python durations method. - Modify test to overload __new__ method instead of just __init__ (Possible breaking change). -Other tweaks and fixes. * Fix: errors in `instruction_properties` and others: - Allow `instruction_properties` method to view optional properties. - Allow `operation_names_for_qargs` to select class instructions when None is passed as a qarg. - Modify __str__ method to display error and duration times as int if the value is 0. - Other tweaks and fixes. * Fix: call `isclass` from rust, instead of passing it from Python. * Fix: Move `update_from_instruction_schedule_map` to rust. * Fix: Move `durations` to rust. * Fix: Move `timing_constraints` to rust * Fix: Move operations_from_name fully to rust * Fix: `instruction_supported` method: - Rewrite the logic of instruction_supported due to previous errors in the method. - Move `check_obj_params` to Rust. - Other tweaks and fixes. * Fix: errors in `from_configuration` class method. - Fix some of the logic when retrieving gates from `name_mapping`. - Remove function arguments in favor of implementing counterpart functions in rust. - Add qubit_props_list_from_props function and return rust datatypes. - Fix wrong error handling procedures when retrieving attributes from backend_property. - Other tweaks and fixes. * Fix: Import `InstructionScheduleMap` directly instead of passing. - `instruction_schedule_map()` now imports the classtype directly from rust instead of needing it to be passed from python. - Remove unused imports in `target.py`. - Ignore unused arguments in `test_extra_props_str`. - Other tweaks and fixes. * Docs: Add docstrings to rust functions - Remove redundant redefinitions in python. - Fix text_signatures for some rust functions. - Added lint exceptions to some necessary imports and function arguments. - Other tweaks and fixes. * Add: Make `Target` and `InstructionProperties` pickleable. - Add `__getstate__` and `__setstate__` methods to make both rust subclasses pickleable. * Fix: Wrong calibration assignment in __setstate__ - Use set_calibration to set the correct calibration argument. - Fix wrong signature in get_non_global_operation_names. - Other tweaks and fixes. * Refactor: HashableVec is now Qarg - Use `PhysicalQubit` instead of u32 for qargs. - Use a `SmallVec` of size 4 instead of a dynamic Vec. - Default to using the `Hash()` method embedded in `SmallVec`. - Add a Default method to easily unwrap Qarg objects. - Other tweaks and fixes. * Add: `get` function to target. - Remove some redundant cloning in code. - Other small fixes. * Fix: Remove unnecessary Optional values in gate_map. - Update every gate_map call to use the new format. - Other small tweaks and fixes. * Refactor: `calibration` is for `InstructionProperties` - Use python `None` instead of option to store `calibration` in `InstructionProperties`. - Adapt code to these changes. - Remove redundant implementation of Hash in Qargs. - Other tweaks and fixes. * Fix: Temporary speedup for `gate_map` access - Added temporary speedups to access the gate_map by returning the values as PyObjects. - Convert qargs to rust tuples instead of initializing a `PyTuple`. - Store `InstructionProperties` as a python ref in gate_map. (Will be changed in future updates). - Other tweaks anf fixes. * Fix: Incorrect extractions for `InstructionProperties` - Fix incorrect conversion of `InstructionProperties` to `Py<InstructionProperties>` - Fix incorrect extraction of qargs in `update_from_instruction_schedule_map` * Fix: Hide all private attributes in `Target` - Hide all private attributes of the `Target` to prevent unecessary cloning. - Other small tweaks and fixes. * Add: New representation of gate_map using new pyclasses: - Make Qarg a sequence pyclass. - Make QargPropsMap the new representation of a GateMap value. - Adapt the code to new structure. - TODO: Add missing magic methods for sequence and mapping objects. - Other small tweaks and fixes. * Add: Use custom datatypes to return values to Python. - Add QargSet datatype to return a set of qargs. - Works as return type for `Target.qargs` - Object is has itertype of QargSetIter. - Rename QargPropMap to PropsMap - Use iterator type IterPropsMap - Other small tweaks and fixes. * Fix: Extend `InstructionProperties` to be subclassable using `__init__: - Made a subclass of `InstructionProperties` that can be extended using an `__init__`method. - Revert previous changes to `test_target.py`. - Other tweaks and fixes. * Refactor: Split target into its own module - Reestructure the files to improve readability of code. - `instruction_properties.rs` contaisn the `InstructionProperties` class. - `mod.rs` contains the `Target` class. - `qargs.rs` contains the Qargs struct to store quantum arguments. - `property_map` contains the Qarg: Property Mapping that will be stored in the gate_map. - Add missing methods to PropsMap: - Add `PropsMapKeys` object to store the qargs as a set. - Add methods to compare and access `PropsMapKey`. - Add QargsOrTuple enum in Qargs to parse Qargs instantly. * Fix: Rest of failing tests in Target - Modify the `InstructionProperties` python wrapper. - InstructionProperties was not assigning properties to rust side. - Make duration in `InstructionProperties` setable. - Add `__eq__` method for `PropMap` to compare with other dicts. - `PropMapKeys` can only be compared with a Set. - Remove `qargs_for_operation_name` from `target.py` - Other small tweaks and fixes. * Add: New GateMap Structure - GateMap is now its own mapping object. - Add `__setstate__` and `__getstate__` methods for `PropMap` and `GateMap`. - Other small tweaks and fixes. * Fix: Make new subclasses pickleable - Add module location to `PropsMap`, `GateMap`, and `Qargs`. - Added default method to PropMap. - Made default method part of class initializers. - Other smalls tweaks and fixes. * Fix: Remove redundant lookup in Target (Qiskit#12373) * Format: `mod.rs` qubit_comparison to one line. * Add: `GateMapKeys` object in GateMap: - Use IndexSet as a base to preserve the insertion order. - Other tweaks and fixes. * Add: __sub__ method to GateMapKeys * Fix: Modify `GateMap` to store values in Python heap. - Fix `GateMap.__iter__` to use an IndexKeys iterator. - Other small tweaks and fixes. * Fix: Remove duplicate import of `IndexSet::into_iter` in `GateMap`. - Make `__iter__` use the keys() method in `GateMap`. * Fix:: Adapt to target changes (Qiskit#12288) - Fix lint stray imports. * Fix: Incorrect creation of parameters in `update_from_instruction_schedule_map` - Add `tupelize` function to create tuples from non-downcastable items. - Fix creation of Parameters by iterating through members of tuple object and mapping them to parameters in `update_from_instruction_schedule_map`. - Add missing logic for creating a Target with/without `qubit_properties`. - Add tuple conversion of `Qargs` to store items in a dict in `BasisTranslator` and `UnitarySynthesis` passes. - Cast `PropsMap` object to dict when comparing in `test_fake_backends.py`. - Modify logic of helper functions that receive a bound object reference, a second `py` not required as an argument. - Add set operation methods to `GateMapKeys`. - Other tweaks and fixes. * Fix: More failing tests - Fix repeated erroneous calls to `add_instruction` in `update_from_instruction_schedule_map` - Add missing condition in `instruction_supported` - Use `IndexSet` instead of `HashSet` for `QargsSet`. - Other small tweaks and fixes. * Add: Macro rules for qargs and other sequences. - Create `QargSet` and `PropsMap` using the new macros. - Return a `TargetOpNames` ordered set to python in `operation_names`. - Remove the Python side `operation_names.` - Fix faulty docstring in `target.py`. - Other tweaks and fixes. * Docs: Add necessary docstrings to all new rust functions. - Remove duplicate Iterator in GateMap. - Other small tweaks and fixes. * Fix: Use `GILOneCell` and remove `Qargs` - Use `GILOneCell` to import python modules only once at initialization. - Remove the custom data structure `Qargs` to avoid conversion overhead. - `Qargs` does not use `PhysicalQubits`, `u32` is used instead. - Fix `__setstate__ `and `__getstate__` methods for `PropsMap`, `GateMap`, and `key_like_set_iterator` macro_rule. - Update code to use the new structures. - TODO: Fix broken tests. * Fix: Cast `Qargs` to `Tuple` in specific situations - Use tupleize to cast `Qargs` to `Tuple` in `instructions`. - Use downcast to extract string in `add_instruction`. - Other tweaks and fixes. * Add: Make `Target` Representable in Rust - Rename `InstructionProperties` as `BaseInstructionProperties`. - Remove `Calibration` from the rust space. - Restore `gate_map`, `coupling_map`, `instruction_schedule_map`, and `instruction_durations` to rust. - Remove all unnecessary data structures from rust space. - Other tweaks and fixes. * Refactor: Remove previour changes to unrelated files. * Add: rust native functions to target - Added rust native functionality to target such that a `py` would not be needed to use one. - Add Index trait to make `Target` subscriptable. - Other small tweaks and fixes. * Fix: Remove all unnecessary python method calls. - Remove uage of `inspect.isclass`. - Rename `Target` to `BaseTarget` in the rust side. - Rename `err.rs` to `errors.rs`. - Remove rust-native `add_inst` and `update_inst` as Target should not be modified from Rust. - Made `add_instruction` and `update_instruction_properties` private in `BaseTarget`. - Add missing `get` method in `Target`. - Other tweaks and fixes * Format: Fix lint * Fix: Wrong return value for `BaseTarget.qargs` * Add: Temporary Instruction representation in rust. - Add temporary instruction representation to avoid repeated extraction from python. * Add: Native representation of coupling graph * Fix: Wrong attribute extraction for `GateRep` * Remove: `CouplingGraph` rust native representation. - Move to different PR. * Format: Remove stray whitespace * Add: `get_non_global_op_names` as a rust native function * Fix: Use Ahash for Hashing - Use ahash for hashing when possible. - Rename `BaseTarget` to `Target` in rust only. - Rename `BaseInstructionProperties` to `InstructionProperties` in rust only. - Remove optional logic from `generate_non_global_op_names`. - Use dict for `__setstate__` and `__getstate__` in `Target`. - Reduced the docstring for `Target` and `InstructionProperties`. - Other small tweaks and fixes. * Format: new changes to `lib.rs` * Format: Adapt to new lint rules * Fix: Use new gates infrastructure (Qiskit#12459) - Create custom enum to collect either a `NormalOperation` or a `VariableOperation` depending on what is needed. - Add a rust native `is_instruction_supported` method to check whether a Target supports a certain instruction. - Make conversion methods from `circuit_instruction.rs` public. - Add comparison methods for `Param` in `operations.rs` - Remove need for isclass method in rustwise `add_instruction` - Other tweaks and fixes. * Format: Fix rust formatting * Add: rust-native method to obtain Operstion objects. * Add: Comparison methods for `Param` * FIx: Add display methods for `Params` * Format: Fix lint test * Format: Wrong merge conflict solve * Fix: Improve rust methods to use iterators. - Adapt the Python methods to leverage the rust native improvements. - Use python native structures for the Python methods. * Format: Remove extra blankspace * Fix: Remove `text_signature`, use `signature` instead. * Fix: Rectify the behavior of `qargs` - Keep insertion order by inserting all qargs into a `PySet`. - Perform conversion to `PyTuple` at insertion time leveraging the iterator architecture. - Remove python side counterpart to avoid double iteration. - Make rust-native `qargs` return an iterator. * Fix: Corrections from Matthew's review - Use `format!` for repr method in `InstructionProperties` - Rename `Variable` variant of `TargetInstruction` to `Variadic`. - Remove `internal_name` attribute from `TargetOperation`. - Remove `VariableOperation` class. - Use `u32` for `granularity`, `pulse_alignment`, and `acquire_alignment`. - Use `Option` to store nullable `concurrent_measurements. - Use `&str` instead of `String` for most function arguments. - Use `swap_remove` to deallocate items from the provided `properties` map in `add_instruction`. - Avoid cloning instructions, use `to_object()` instead. - Avoid using `.to_owned()`, use `.clone()` instead. - Remove mention of `RandomState`, use `ahash::HashSet` instead. - Move parameter check to python in `instruction_supported`. - Avoid exposing private attributes, use the available ones instead. - Filter out `Varidadic` Instructions as they're not supported in rust. - Use peekable iterator to peak at the next qargs in `generate_non_global_op_names`. - Rename `qarg_set` to `deduplicated_qargs` in `generate_non_global_op_names`. - Return iterator instances instead of allocated `Vec`. - Add `python_compare` and `python_is_instance` to perform object comparison with objects that satisfy the `ToPyObject` trait. - Other small tweaks and fixes. * Implement a nullable dict-like structure for IndexMap (Qiskit#2) * Initial: Implement a nullable dict-like structure for IndexMap * FIx: Erroneous item extraction from Python - Fix error that caused `None` values to be ignored from `None` keys. - Removed mutability from rust function argument in `add_instruction`. - Object is mutably referenced after option unwrapping. - Add missing header in `nullable_index_map.rs`. - Add Clone as a `K` and/or `V` constraint in some of the iterators. - Remove `IntoPy` constraint from `NullableIndexMap<K, V>`. - Add `ToPyObject` trait to `NullableIndexMap<K, V>`. * Fix: inplace modification of Python dict. - Perform `None` extraction from rust. - Revert changes to `Target.py` * Fix: Avoid double iteration by using filter_map. * Docs: Add inline comments. * Fix: More specific error message in `NullableIndexMap` * Fix: Use `Mapping` as the metaclass for `Target` - Minor corrections from Matthew's review. * Fix: Make `Target` crate-private. - Due to the private nature of `NullableIndexMap`, the `Target` has to be made crate private. - Add temporary`allow(dead_code)` flag for the unused `Target` and `NullableIndexMap` methods. - Fix docstring of `Target` struct. - Fix docstring of `add_instruction`. - Make several python-only operations public so they can be used with other `PyClass` instances as long as they own the gil. - Modify `py_instruction_supported` to accept bound objects. - Use rust-native functions for some of the instance properties. - Rewrite `instruction` to return parameters as slice. - `operation_names` returns an `ExactSizeIterator`. - All rust-native methods that return an `OperationType` object, will return a `NormalOperation` instance which includes the `OperationType` and the parameters. * Fix: Comments from Matthew's review - Mention duplication in docstring for rust Target. - Use f"{*:g}" to avoid printing the floating point for 0 in `Target`'s repr method. - Add note mentioning future unit-tests in rust. * Fix: Adapt to Qiskit#12730
raynelfss
added a commit
to raynelfss/qiskit
that referenced
this pull request
Aug 1, 2024
raynelfss
added a commit
to raynelfss/qiskit
that referenced
this pull request
Aug 2, 2024
raynelfss
added a commit
to raynelfss/qiskit
that referenced
this pull request
Aug 2, 2024
raynelfss
added a commit
to raynelfss/qiskit
that referenced
this pull request
Aug 6, 2024
raynelfss
added a commit
to raynelfss/qiskit
that referenced
this pull request
Aug 29, 2024
raynelfss
added a commit
to raynelfss/qiskit
that referenced
this pull request
Aug 30, 2024
raynelfss
added a commit
to raynelfss/qiskit
that referenced
this pull request
Sep 5, 2024
github-merge-queue bot
pushed a commit
that referenced
this pull request
Sep 25, 2024
* Initial: Add equivalence to `qiskit._accelerate.circuit` * Add: `build_basis_graph` method * Add: `EquivalencyLibrary` to `qiskit._accelerate.circuit` - Add `get_entry` method to obtain an entry from binding to a `QuantumCircuit`. - Add `rebind_equiv` to bind parameters to `QuantumCircuit` * Add: PyDiGraph converter for `equivalence.rs` * Add: Extend original equivalence with rust representation * Fix: Correct circuit parameter extraction * Add: Stable infrastructure for EquivalenceLibrary - TODO: Make elements pickleable. * Add: Default methods to equivalence data structures. * Fix: Adapt to new Gate Structure * Fix: Erroneous display of `Parameters` * Format: Fix lint test * Fix: Use EdgeReferences instead of edge_indices. - Remove stray comment. - Use match instead of matches!. * Fix: Use StableDiGraph for more stable indexing. - Remove required py argument for get_entry. - Reformat `to_pygraph` to use `add_nodes_from` and `add_edges_from`. - Other small fixes. * Fix: Use `clone` instead of `to_owned` - Use `clone_ref` for the PyObject Graph instance. * Fix: Use `OperationTypeConstruct` instead of `CircuitInstruction` - Use `convert_py_to_operation_type` to correctly extract Gate instances into rust operational datatypes. - Add function `get_sources_from_circuit_rep` to not extract circuit data directly but only the necessary data. - Modify failing test due to different mapping. (!!) - Other tweaks and fixes. * Fix: Elide implicit lifetime of PyRef * Fix: Make `CircuitRep` attributes OneCell-like. - Attributes from CircuitRep are only written once, reducing the overhead. - Modify `__setstate__` to avoid extra conversion. - Remove `get_sources_from_circuit_rep`. * Fix: Incorrect pickle attribute extraction * Remove: Default initialization methods from custom datatypes. - Use `__getnewargs__ instead. * Remove: `__getstate__`, `__setstate__`, use `__getnewargs__` instead. * Fix: Further improvements to pickling - Use python structures to avoid extra conversions. - Add rust native `EquivalenceLibrary.keys()` and have the python method use it. * Fix: Use `PyList` and iterators when possible to skip extra conversion. - Use a `py` token instead of `Python::with_gil()` for `rebind_params`. - Other tweaks and fixes. * Fix: incorrect list operation in `__getstate__` * Fix: improvements on rust native methods - Accept `Operations` and `[Param]` instead of the custom `GateOper` when calling from rust. - Build custom `GateOper` inside of class. * Remove: `add_equiv`, `set_entry` from rust-native methods. - Add `node_index` Rust native method. - Use python set comparison for `Param` check. * Remove: Undo changes to Param - Fix comparison methods for `Key`, `Equivalence`, `EdgeData` and `NodeData` to account for the removal of `PartialEq` for `Param`. * Fix: Leverage usage of `CircuitData` for accessing the `QuantumCircuit` intructions in rust. - Change implementation of `CircuitRef, to leverage the use of `CircuitData`. * Add: `data()` method to avoid extracting `CircuitData` - Add `py_clone` to perform shallow clones of a `CircuitRef` object by cloning the references to the `QuantumCircuit` object. - Extract `num_qubits` and `num_clbits` for CircuitRep. - Add wrapper over `add_equivalence` to be able to accept references and avoid unnecessary cloning of `GateRep` objects in `set_entry`. - Remove stray mutability of `entry` in `set_entry`. * Fix: Make `graph` attribute public. * Fix: Make `NoteData` attributes public. * Fix: Revert reference to `CircuitData`, extract instead. * Add: Make `EquivalenceLibrary` graph weights optional. * Fix: Adapt to #12730 * Fix: Use `IndexSet` and `IndexMap` * Fix: Revert changes from previously failing test * Fix: Adapt to #12974 * Fix: Use `EquivalenceLibrary.keys()` instead of `._key_to_node_index` * Chore: update dependencies * Refactor: Move `EquivalenceLibrary` to `_accelerate`. * Fix: Erroneous `pymodule` function for `equivalence`. * Fix: Update `EquivalenceLibrary` to store `CircuitData`. - The equivalence library will now only store `CircuitData` instances as it does not need to call python directly to re-assign parameters. - An `IntoPy<PyObject>` trait was adapted so that it can be automatically converted to a `QuantumCircuit` instance using `_from_circuit_data`. - Updated all tests to use register-less qubits in circuit comparison. - Remove `num_qubits` and `num_clbits` from `CircuitRep`. * Fix: Make inner `CircuitData` instance public. * Fix: Review comments and ownership issues. - Add `from_operation` constructor for `Key`. - Made `py_has_entry()` private, but kept its main method public. - Made `set_entry` more rust friendly. - Modify `add_equivalence` to accept a slice of `Param` and use `Into` to convert it into a `SmallVec` instance. * Fix: Use maximum possible integer value for Key in basis_translator. - Add method to immutably borrow the `EquivalenceLibrary`'s graph. * Fix: Use generated string, instead of large int - Using large int as the key's number of qubits breaks compatibility with qpy, use a random string instead. --------- Co-authored-by: John Lapeyre <[email protected]>
raynelfss
added a commit
to raynelfss/qiskit
that referenced
this pull request
Sep 26, 2024
github-merge-queue bot
pushed a commit
that referenced
this pull request
Oct 7, 2024
…r` to rust. (#12811) * Add: Basis search function - Add rust counterpart for `basis_search`. - Consolidated the `BasisSearchVisitor` into the function due to differences in rust behavior. * Fix: Wrong return value for `basis_search` * Fix: Remove `IndexMap` and duplicate declarations. * Fix: Adapt to #12730 * Remove: unused imports * Docs: Edit docstring for rust native `basis_search` * Fix: Use owned Strings. - Due to the nature of `hashbrown` we must use owned Strings instead of `&str`. * Add: mutable graph view that the `BasisTranslator` can access in Rust. - Remove import of `random` in `basis_translator`. * Fix: Review comments - Rename `EquivalenceLibrary`'s `mut_graph` method to `graph_mut` to keep consistent with rust naming conventions. - Use `&HashSet<String>` instead of `HashSet<&str>` to avoid extra conversion. - Use `u32::MAX` as num_qubits for dummy node. - Use for loop instead of foreachj to add edges to dummy node. - Add comment explaining usage of flatten in `initialize_num_gates_remain_for_rule`. - Remove stale comments. * Update crates/accelerate/src/basis/basis_translator/basis_search.rs --------- Co-authored-by: Matthew Treinish <[email protected]>
ElePT
pushed a commit
to ElePT/qiskit
that referenced
this pull request
Oct 9, 2024
* Initial: Add equivalence to `qiskit._accelerate.circuit` * Add: `build_basis_graph` method * Add: `EquivalencyLibrary` to `qiskit._accelerate.circuit` - Add `get_entry` method to obtain an entry from binding to a `QuantumCircuit`. - Add `rebind_equiv` to bind parameters to `QuantumCircuit` * Add: PyDiGraph converter for `equivalence.rs` * Add: Extend original equivalence with rust representation * Fix: Correct circuit parameter extraction * Add: Stable infrastructure for EquivalenceLibrary - TODO: Make elements pickleable. * Add: Default methods to equivalence data structures. * Fix: Adapt to new Gate Structure * Fix: Erroneous display of `Parameters` * Format: Fix lint test * Fix: Use EdgeReferences instead of edge_indices. - Remove stray comment. - Use match instead of matches!. * Fix: Use StableDiGraph for more stable indexing. - Remove required py argument for get_entry. - Reformat `to_pygraph` to use `add_nodes_from` and `add_edges_from`. - Other small fixes. * Fix: Use `clone` instead of `to_owned` - Use `clone_ref` for the PyObject Graph instance. * Fix: Use `OperationTypeConstruct` instead of `CircuitInstruction` - Use `convert_py_to_operation_type` to correctly extract Gate instances into rust operational datatypes. - Add function `get_sources_from_circuit_rep` to not extract circuit data directly but only the necessary data. - Modify failing test due to different mapping. (!!) - Other tweaks and fixes. * Fix: Elide implicit lifetime of PyRef * Fix: Make `CircuitRep` attributes OneCell-like. - Attributes from CircuitRep are only written once, reducing the overhead. - Modify `__setstate__` to avoid extra conversion. - Remove `get_sources_from_circuit_rep`. * Fix: Incorrect pickle attribute extraction * Remove: Default initialization methods from custom datatypes. - Use `__getnewargs__ instead. * Remove: `__getstate__`, `__setstate__`, use `__getnewargs__` instead. * Fix: Further improvements to pickling - Use python structures to avoid extra conversions. - Add rust native `EquivalenceLibrary.keys()` and have the python method use it. * Fix: Use `PyList` and iterators when possible to skip extra conversion. - Use a `py` token instead of `Python::with_gil()` for `rebind_params`. - Other tweaks and fixes. * Fix: incorrect list operation in `__getstate__` * Fix: improvements on rust native methods - Accept `Operations` and `[Param]` instead of the custom `GateOper` when calling from rust. - Build custom `GateOper` inside of class. * Remove: `add_equiv`, `set_entry` from rust-native methods. - Add `node_index` Rust native method. - Use python set comparison for `Param` check. * Remove: Undo changes to Param - Fix comparison methods for `Key`, `Equivalence`, `EdgeData` and `NodeData` to account for the removal of `PartialEq` for `Param`. * Fix: Leverage usage of `CircuitData` for accessing the `QuantumCircuit` intructions in rust. - Change implementation of `CircuitRef, to leverage the use of `CircuitData`. * Add: `data()` method to avoid extracting `CircuitData` - Add `py_clone` to perform shallow clones of a `CircuitRef` object by cloning the references to the `QuantumCircuit` object. - Extract `num_qubits` and `num_clbits` for CircuitRep. - Add wrapper over `add_equivalence` to be able to accept references and avoid unnecessary cloning of `GateRep` objects in `set_entry`. - Remove stray mutability of `entry` in `set_entry`. * Fix: Make `graph` attribute public. * Fix: Make `NoteData` attributes public. * Fix: Revert reference to `CircuitData`, extract instead. * Add: Make `EquivalenceLibrary` graph weights optional. * Fix: Adapt to Qiskit#12730 * Fix: Use `IndexSet` and `IndexMap` * Fix: Revert changes from previously failing test * Fix: Adapt to Qiskit#12974 * Fix: Use `EquivalenceLibrary.keys()` instead of `._key_to_node_index` * Chore: update dependencies * Refactor: Move `EquivalenceLibrary` to `_accelerate`. * Fix: Erroneous `pymodule` function for `equivalence`. * Fix: Update `EquivalenceLibrary` to store `CircuitData`. - The equivalence library will now only store `CircuitData` instances as it does not need to call python directly to re-assign parameters. - An `IntoPy<PyObject>` trait was adapted so that it can be automatically converted to a `QuantumCircuit` instance using `_from_circuit_data`. - Updated all tests to use register-less qubits in circuit comparison. - Remove `num_qubits` and `num_clbits` from `CircuitRep`. * Fix: Make inner `CircuitData` instance public. * Fix: Review comments and ownership issues. - Add `from_operation` constructor for `Key`. - Made `py_has_entry()` private, but kept its main method public. - Made `set_entry` more rust friendly. - Modify `add_equivalence` to accept a slice of `Param` and use `Into` to convert it into a `SmallVec` instance. * Fix: Use maximum possible integer value for Key in basis_translator. - Add method to immutably borrow the `EquivalenceLibrary`'s graph. * Fix: Use generated string, instead of large int - Using large int as the key's number of qubits breaks compatibility with qpy, use a random string instead. --------- Co-authored-by: John Lapeyre <[email protected]>
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
This is a large overhaul of how circuit instructions are both stored in Rust (
PackedInstruction) and how they are presented to Python (CircuitInstruction). In summary:The old
OperationTypeenum is now collapsed into a manually managedPackedOperation. This is logically equivalent, but stores aPyGate/PyInstruction/PyOperationindirectly through a boxed pointer, and stores aStandardGateinline. As we expect the vast majority of gates to be standard, this hugely reduces the memory usage. The enumeration is manually compressed to a single pointer, hiding the discriminant in the low, alignment-required bytes of the pointer.PackedOperation::view()unpacks the operation into a proper reference-like enumerationOperationRef<'a>, which implementsOperation(though there is also atry_standard_gatemethod to get the gate without unpacking the whole enumeration).Both
PackedInstructionandCircuitInstructionuse thisPackedOperationas the operation storage.PackedInstructionis now completely the Rust-space format for data, andCircuitInstructionis purely for communication with Python.On my machine, this commit brings the utility-scale benchmarks to within 10% of the runtime of 1.1.0 (and some to parity), despite all the additional overhead.
Changes to accepting and building Python objects
A
PackedInstructionis created by copy constructor from aCircuitInstructionbyCircuitData::pack. There is nopack_owned(really, there never was - the previous method didn't take ownership) because there's never ownedCircuitInstructions coming in; they're Python-space interop, so we never own them (unless we clone them) other than when we're unpacking them.PackedInstructionis currently just created manually when not coming from aCircuitInstruction. It's not hard, and makes it easier to re-use known intern indices than to waste time re-interning them. There is no need to go viaCircuitInstruction.CircuitInstructionnow has two separated Python-space constructors: the old one, which is the default and takes(operation, qubits, clbits)(and extracts the information), and a new fast-pathfrom_standardwhich asks only for the standard gate, qubits and params, avoiding operator construction.To accept a Python-space operation, extract a Python object to
OperationFromPython. This extracts the components that are separate in Rust space, but joined in Python space (the operation, params and extra attributes). This replacesOperationInputandOperationTypeConstruct, being more efficient at the extraction, including providing the data in the formats needed forPackedInstructionorCircuitInstruction.To retrieve the Python-space operation, use
CircuitInstruction::get_operationorPackedInstruction::unpack_py_opas appropriate. Both will cache and reuse the op, ifcache_pygatesis active. (Though note that if the op is created byCircuitInstruction, it will not propagate back to aPackedInstruction.)Avoiding operation creation
The
_raw_opfield ofCircuitInstructionis gone, becausePyGate,PyInstructionandPyOperationare no longer pyclasses and no longer exposed to Python. Instead, we avoid operation creation by:having an internal
DAGNode::_to_circuit_instruction, which returns a copy of the internalCircuitInstruction, which can then be used withCircuitInstruction.replace, etc.having
CircuitInstruction::is_standard_gateto query from Python space if we should bother to create the operator.changing
CircuitData::map_opstomap_nonstandard_ops, and having it only call the Python callback function if the operation is not an unconditional standard gate.Memory usage
Given the very simple example construction script:
This uses 1.5GB in max resident set size on my Macbook (note that it's about 12 million gates) on both 1.1.0 and with this commit, so we've undone our memory losses. The parent of this commit uses 2GB.
However, we're in a strong position to beat 1.1.0 in the future now; there are two obvious large remaining costs:
There are 16 bytes per
PackedInstructionfor the Python-operation caching (worth about 180MB in this benchmark, since no Python operations are actually created).There is also significant memory wastage in the current
SmallVec<[Param; 3]>storage of the parameters; for all standard gates, we know statically how many parameters are / should be stored, and we never need to increase the capacity. Further, theParamenum is 16 bytes wide per parameter, of which nearly 8 bytes is padding, but for all our current use cases, we only care if all the parameters or floats (for everything else, we're going to have to defer to Python). We could move the discriminant out to the level of the parameters structure, and save a large amount of padding.Further work
There's still performance left on the table here:
We still copy-in and copy-out of
CircuitInstructiontoo much right now; we might want to make all theCircuitInstructionfields nullable and haveCircuitData::appendtake them by move rather than by copy.The qubits/clbits interner requires owned arrays going in, but most interning should return an existing entry. We probably want to switch to have the interner take references/iterators by default, and clone when necessary. We could have a small circuit optimisation where the intern contexts reserve the first n entries to use for an all-to-all connectivity interning for up to (say) 8 qubits, since the transpiler will want to create a lot of ephemeral small circuits.
The
Paramvectors are too heavy at the moment;SmallVec<[Param; 3]>is 56 bytes wide, despite the vast majority of gates we care about having at most one single float (8 bytes). Dead padding is a large chunk of the memory use currently.Details and comments
Benchmarks in follow-up comments because GitHub complained I went over the limit. Top-level summary:
QuantumCircuitcreation and manipulation, and has non-trivial improvements forDAGCircuitmanipulationDAGCircuitto Rust #12550 (or at least the subsequent porting of transpiler passes) should get us beyond 1.1.0 performance.assign_parametersis still currently taking a big hit, but moving the orchestration of that down to Rust would likely get us back to parity.Closes:
CircuitInstructionandPackedInstruction#12622OperationType#12567