* Make dense layout algorithm deterministic when run in parallel
The dense layout algorithm is trying to find the densest k subgraph of
the connectivity graph. To do this it performs a BFS from each node in
the in graph of k nodes to determine the subgraph with the most number
of edges. But in cases of ties where there are subgraphs with the same
number of edges the exact output would be determined by the iteration
order that we're evaluating a BFS search. However, this algorithm runs
in parallel in most cases and the exact iteration order isn't going to
be stable when running in parallel. It will depend on which threads
finish first. This commit fixes this potential non-determinism in the
algorithm by defaulting to the lower node index's trial results instead
of relying on the execution order. This should mean we return identical
results regardless of how many threads are run or how quickly they
execute.
* Add release note
* Also handle the use_error case too
* Add PyInstruction::blocks method.
This gives us a way to get the blocks of a control flow
operation as an iterator of CircuitData. If called on
a non-control-flow instruction, returns None.
This is not intended as a final API for this, which
will likely instead return an iterator of &CircuitData
once control flow operations have been fully ported to
Rust and own their blocks without requiring conversion.
* Expose blocks at Operation level.
Also addresses other review comments.
* Add comment to explain unwraps.
Since 8e5fab6c (gh-12959), this branch has been dead, and the Rust
method alluded to in it (`compute_error_one_qubit_sequence`) was
removed. This is occasionally - but for some reason not always -
detected by `pylint`, and it's a true error so should be removed.
In fact, almost all of the Python code in `Optimize1qDecomposition` is
dead from the perspective of that transpiler pass. However,
`Optimize1qCommutation` has no concept of safe API boundaries, and is in
fact still using all of these methods (and the default
`UnitarySynthesis` plugin does a little too). A follow-up commit can
reorganise the code to fix this, but this commit is intended to be
mergeable faster, to unblock a couple of PRs that are triggering the
pylint failure.
We previously exposed the data attribute `STDGATES_INC_GATES`, but its
contents are all `CustomGate`, which can't be inspected programmatically
from Python space. This makes it so you can query _what_ the gates,
their number of parameters, and how to construct them.
Swapping the behaviour of the exporter to error on unknown includes was
a mistake, and a breaking change from previous versions of the OpenQASM
3 exporter, since the `basis_gates` argument can be used to emulate the
expected behaviour of this option, even though the previously documented
behaviour was never actually fulfilled.
This restores the previous non-erroring behaviour, and corrects the
documentation to note how the ``includes`` argument should be used in
the near term.
* Initial: Add rust-native `apply_operation_front` and `apply_operation_back methods.
- These methods allow rust users to add an operation to either the front or the back of the circuit without needing to have valid interned indices for the Qargs/Qargs of said operations.
- Leverage the new methods in the existing python variants of them.
* Fix: Remove extra blank space
* Fix: Revert changes to python `apply_op` methods.
- To avoid an ownership issue, usage of the new methods from the Python variants has been removed.
- As requested, the insertion check in the rust `apply_op` has been removed.
* Fix: Adapt to new `ExtraInstructionProperties` infrastructure.
* Format: Change redundant iteration statement for params
* Add LightSABRE citation to sabre documentation
We recently published a pre-print of a new paper that describes all the
improvements we've developed ontop of the original SABRE algorithm as
part of Qiskit's implementation. This commit adds a citation for that
new paper to the documentation of the two sabre passes in qiskit to
better document how the pass works in practice.
* Add missing citation to SabrePreLayout and fix formatting in SabreSwap
* Tweak wording in sabre swap docstring
* add pyfunction for decompose_two_qubit_product_gate
* replace python code by rust for decompose_two_qubit_product_gate
* remove unused code from two_qubit_decompose.py
* updates following review comments
* port trace_to_fid to rust
* port Ud to rust
* fix lint errors
* fix lint errors
* format
* remove PyResult following review
* Move Option<Box<_>> into ExtraInstructionAttributes.
Previously, CircuitInstruction and PackedInstruction held
the ExtraInstructionAttributes struct within an Option<Box<_>>.
By putting the Option<Box<_>> inside ExtraInstructionAttributes,
we can use the struct itself to manage its memory and provide
access to the attributes behind a nicer interface.
The size of ExtraInstructionAttributes should be the same size
as the old Option<Box<ExtraInstructionAttributes>>, so this
should not have memory implications.
* Address review comments.
- Use tuple struct.
- Use 'Attributes' over 'Attrs'.
- Add doc comment for internal 'ExtraAttributes' struct.
- Add doc comments for methods.
- Add setters for unit and duration.
* Fix performance regression from unnecessary dict creation.
* Oxidize dag_to_circuit()
This commit migrates the dag_to_circuit() converter function
implementation to rust. The core of this is having a DAGCircuit to
CircuitData converter function in rust. To do this efficiently we just
copy the BitDatas and Interners to the new circuit data object being
created and then just iterate over all the packed instructions in the
circuit. To facilitate this a new constructor for CircuitData is added
that takes an iterator of PackedInstructions and the BitDatas and
Interners and builds a new circuit data from that.
* Apply suggestions from code review
Co-authored-by: Raynel Sanchez <87539502+raynelfss@users.noreply.github.com>
* Update crates/circuit/src/converters.rs
Co-authored-by: Raynel Sanchez <87539502+raynelfss@users.noreply.github.com>
* Update crates/circuit/src/converters.rs
Co-authored-by: Raynel Sanchez <87539502+raynelfss@users.noreply.github.com>
* Use immutable access methods instead of relying on public fields
* Add docstring to the new CircuitData constructor method
---------
Co-authored-by: Raynel Sanchez <87539502+raynelfss@users.noreply.github.com>
Now that the 1.3.0b1 pre-release has been published this commit reverts
the version number back to 1.3.0 to differentiate development builds
from the published pre-release.
* Make DAGCircuit fields 'dag', 'qubits', and 'clbits' private.
Immutable getters are added to provide the same access we were using
without the risk of data incoherence.
* Update commutation_cancellation.rs.
We recently tried to publish the 1.3.0b1 release unsucessfully. During
the wheel build jobs abi3audit was failing because it correctly
identified a couple of abi violations in the builds. The first was we
were tagging the package incorrectly as being with a minimum level of
Python 3.8 instead of the correct minimum of 3.9. This commit updates
the setup.py tag to match the package metadata with the binary.
The second issue is we were creating `PyInit_*` symbols for submodules
which goes against the abi3 naming recomendations and abi3 audit flags
it as a violation. We had this issue previously in 1.2.0 but was fixed
for that release. However since 1.2.0 and now several stray usages of
`#[pymodule]` on submodules slipped in during development. This commit
fixes those so the bad symbol names won't be created anymore.
Once this commit merges we should tag the new commit as 1.3.0b1.
Co-authored-by: Raynel Sanchez <87539502+raynelfss@users.noreply.github.com>
* Prepare 1.3.0b1 release
This commit changes the version strings from 1.3.0 to 1.3.0b1 in
preparation for tagging the 1.3.0b1 pre-release. After we publish the
1.3.0b1 tag we should switch the version number back to 1.3.0 to
continue developing the 1.3.0 release series as we approach the actual
release.
* Also skip 3.8 builds on 32 bit platforms
In the recently merged #12910 the minimum Python version was raised to
3.9. As part of this the cibuildwheel config was updated to skip 38
uilds since they wont work anymore. However that PR missed that there
was an additional skip config in for the 32 bit builds in the job
definition. This commit adds the missing skip so the 32bit wheel builds
will work as well.
* Fix edge case in Sabre release valve
If a Sabre trial does not find a set of Swaps to route nodes, the "release valve" adds Swaps to route the two-qubit gate in between the the closest two qubits. In rare cases, this leads to _more_ than one gate being routable, which was not handled correctly previously.
Co-authored-by: Jake Lishman <jake.lishman@ibm.com>
* add reno and test
* Use sensible syntax
Co-authored-by: Kevin Hartman <kevin@hart.mn>
* Use sensible grammar
Co-authored-by: Kevin Hartman <kevin@hart.mn>
* clippy
* Check if the new test breaks CI
* Revert "Check if the new test breaks CI"
This reverts commit 01bcdc7ca2.
---------
Co-authored-by: Jake Lishman <jake.lishman@ibm.com>
Co-authored-by: Kevin Hartman <kevin@hart.mn>
* Raise minimum python version to 3.9
Qiskit 1.2.0 was the final minor version release of qiskit with Python
3.8 support. As Python 3.8 is going EoL before the next Qiskit minor
release 1.3.0 we can drop support for running with 3.8 on the main
branch now. This commit makes that change and updates everything using
python 3.8 currently to use our new minimum instead.
* Pin dependency versions when installing historical versions
Now that we're running Python 3.9 as the base version we are
encountering compatibility issues with some of our dependencies because
3.9 supports newer versions of things like numpy than older Qiskit
releases were compatible with. The only way to work around this is to
pin the versions when installing historical versions. To start this sets
a single constraints version since we can get away with using the same
version for everything. In the future though it is possible that we'll
need separate files for different historical releases.
* Update reference images
* Drop 3.8 support in asv too
* Remove deprecation warning for 3.8
* Initial: Add add_from_iter method to DAGCircuit
- Introduce a method that adds a chain of `PackedInstruction` continuously avoiding the re-linking of each bit's output-node until the very end of the iterator.
- TODO: Add handling of vars
- Add header for a `from_iter` function that will create a `DAGCircuit` based on a chain of `PackedInstruction`.
* Fix: leverage new methods in layers
- Fix incorrect re-insertion of last_node.
* Fix: Keep track of Vars for add_from_iter
- Remove `from_iter`
* Fix: Incorrect modification of last nodes in `add_from_iter`.
- Use `entry` api to either modify or insert a value if missing.
* Fix: Cycling edges in when adding vars.
- A bug that adds duplicate edges to vars has been temporarily fixed. However, the root of this problem hasn't been found yet. A proper fix is pending. For now skip those instances.
* Fix: Remove set collecting all nodes to be connected.
- A set collecting all the new nodes to connect with a new node was preventing additional wires to connect to subsequent nodes.
* Fix: Adapt to #13033
* Refactor: `add_from_iter` is now called `extend` to stick with `Rust` nomenclature.
* Fix: Remove duplicate vars check
* Fix: Corrections from code review.
- Use Entry API to modify last nodes in the var.
- Build new_nodes with an allocated vec.
- Add comment explaining the removal of the edge between the output node and its predecessor.
* Fix: Improper use of `Entry API`.
- Use `or_insert_with` instead of `or_insert` to perform actions before inserting a value.
* Initial: Add add_from_iter method to DAGCircuit
- Introduce a method that adds a chain of `PackedInstruction` continuously avoiding the re-linking of each bit's output-node until the very end of the iterator.
- TODO: Add handling of vars
- Add header for a `from_iter` function that will create a `DAGCircuit` based on a chain of `PackedInstruction`.
* Initial: Expose `CircuitData` interners and registers to the `qiskit-circuit` crate.
- Make the `CircuitData` iter method be an exact-size iterator.
* FIx: Expose immutable views of interners, registers and global phase.
- Revert the changes making the interners and registers visible to the crate `qiskit-circuit`.
- Create methods to expose immutable borrowed views of the interners, registers and global_phase to prevent from mutating the DAGCircuit.
- Add `get_qargs` and `get_cargs` to unpack interned qargs ans cargs.
- Other tweaks and fixes.
* Refactor: Use naming convention for getters.
* Docs: Apply suggestions from code review
- Correct incorrect docstrings for `qubits()` and `clbits()`
Co-authored-by: Eli Arbel <46826214+eliarbel@users.noreply.github.com>
* Initial: Add `circuit_to_dag` in rust.
- Add new method `DAGCircuit::from_quantum_circuit` which uses the data from a `QuantumCircuit` instance to build a dag_circuit.
- Expose the method through a `Python` interface with `circuit_to_dag` which goes by the alias of `core_circuit_to_dag` and is called by the original method.
- Add an arbitrary structure `QuantumCircuitData` that successfully extract attributes from the python `QuantumCircuit` instance and makes it easier to access in rust.
- This structure is for attribute extraction only and is not clonable/copyable.
- Expose a new module `converters` which should store all of the rust-side converters whenever they get brought into rust.
- Other small tweaks and fixes.
* Fix: Move qubit ordering check to python
- Add more accurate estimate of num_edges.
* Fix: Regenerate `BitData` instances dynamically instead of cloning.
- When converting from `QuantumCircuit` to `DAGCircuit`, we were previously copying the instances of `BitData` by cloning. This would result in instances that had extra qubits that went unused. This commit fixes this oversight and reduced the amount of failing times from 160 to 12.
* Fix: Make use of `copy_operations`
- Use `copy_operations` to copy all of the operations obtained from `CircuitData` by calling deepcopy.
- Initialize `._data` manually for instances of `BlueprintCircuit` by calling `._build()`.
- Other small fixes.
* FIx: Adapt to 13033
* Fix: Correctly map qubits and clbits to the `DAGCircuit`.
- Previously, the binding of qubits and clbits into bitdata was done based on the `push_back()` function behavior. This manual mapping was removed in favor of just inserting the qubits/clbits using the `add_qubits()` and `add_clbits()` methods and keeping track of the new indices.
- Remove cloning of interners, use the re-mapped entries instead.
- Remove manual re-mapping of qubits and clbits.
- Remove cloning of BitData, insert qubits directly instead.
- Other small tweaks and fixes.
* Add: `DAGCircuit` from `CircuitData`
- Make `QuantumCircuitData` extraction struct private.
- Rename `DAGCircuit::from_quantum_circuit` into `DAGCircuit::from_circuit` to make more generalized.
- Pass all attributes of `QuantumCircuit` that are passed from python as arguments to the `DAGCircuit::from_circuit` function.
- Add `DAGCircuit::from_circuit_data` as a wrapper to `from_circuit` to create an instance solely from the properties available in `CircuitData`.
- Use `DAGCircuit::from_circuit` as base for `DAGCircuit::from_circuit_data`.
* Fix: Make `QuantumCircuitData` public.
- `QuantumCircuitData` is an extractor helper built only to extract attributes from a Python-based `Quantumircuit` that are not yet available in rust + the qualities that already are through `CircuitData`.
- Add `Clone` trait `QuantumCircuitData` as all its properties are clonable.
- Make `circuit_to_dag` public in rust in case it needs to be used for a transpiler pass.
- Adapt to renaming of `DAGCircuit::add_from_iter` into `DAGCircuit::extend`.
* Fix: Use `intern!` when extracting attributes from Python.
* Fix: Copy instructions in place instead of copying circuit data.
- Use `QuantumCircuitData` as argument for `DAGCircuit::from_circuit`.
- Create instance of `QuantumCircuitData` in `DAGCircuit::from_circuit_data`.
- Re-add duplicate skip in extend.
- Other tweaks and fixes.
* Fix: Remove second re-mapping of bits
- Remove second re-mapping of bits, perform this mapping at insertion of qubits instead.
- Modify insertion of qubits to re-map the indices and store them after insertion should a custom ordering be provided.
- Remove extra `.and_modify()` for `clbits_last_node` from `DAGCircuit::extend`.
- Remove submodule addition to `circuit` module, submodule is now under `_accelerate.converters`.
- Other tweaks and fixes.
* Update crates/circuit/src/dag_circuit.rs
Co-authored-by: Matthew Treinish <mtreinish@kortar.org>
---------
Co-authored-by: Eli Arbel <46826214+eliarbel@users.noreply.github.com>
Co-authored-by: Matthew Treinish <mtreinish@kortar.org>
* fix
* fmt
* comments from code review
* comments from code review
* Update lib.rs
* Apply suggestions from code review
Co-authored-by: Matthew Treinish <mtreinish@kortar.org>
* code review
* Fix rustfmt
* Don't change interface for DAGCircuit
Previously this PR was making the op_names field of the DAGCircuit
struct public so it was accessible to the new transpiler pass code.
However, this opened up the possibility of mutating the field by
mistake, instead this makes the field private again and adds a no-copy
method to get an immutable reference to the field. Additionally, there
were some interface changes made to one method in DAGCircuit that were
not needed anymore, which this reverts to minimize the diff.
* Remove last .first().unwrap()
---------
Co-authored-by: Matthew Treinish <mtreinish@kortar.org>
* Fully port Split2QUnitaries to rust
This commit builds off of #13013 and the other data model in Rust
infrastructure and migrates the InverseCancellation pass to
operate fully in Rust. The full path of the transpiler pass now never
leaves Rust until it has finished modifying the DAGCircuit. There is
still some python interaction necessary to handle parts of the data
model that are still in Python, mainly for creating `UnitaryGate`
instances and `ParameterExpression` for global phase. But otherwise
the entirety of the pass operates in rust now.
This is just a first pass at the migration here, it moves the pass to
use loops in rust. The next steps here are to look at operating
the pass in parallel. There is no data dependency between the
optimizations being done for different gates so we should be able to
increase the throughput of the pass by leveraging multithreading to
handle each gate in parallel. This commit does not attempt
this though, because of the Python dependency and also the data
structures around gates and the dag aren't really setup for
multithreading yet and there likely will need to be some work to
support that.
Part of #12208
* Update pass logic with changes from #13095
Some of the logic inside the Split2QUnitaries pass was updated in a
recently merged PR. This commit makes those changes so the rust
implementation matches the current state of the previous python version.
* Use op_nodes() instead of topological_op_nodes()
* Use Fn trait instead of FnMut for callback
We don't need the callback to be mutable currently so relax the trait to
just be `Fn` instead of `FnMut`. If we have a need for a mutable
environment callback in the future we can change this easily enough
without any issues.
* Avoid extra edge operations in replace_on_incoming_qubits
* Rename function
* Add equivalence-library rules between `rzz` and `cp`
These gates are locally equivalence (as are all the Ising-interaction
gates), and this simple additional rule lets things like QFT, which are
defined by Qiskit's default constructor in terms of `cp`, get converted
into `rzz` or `rzx`.
One `ControlledGate` test needed a case removing, because the underlying
control mechanism now works correctly.
* Rewrite release note
* First working implementation
* A control flow fix, lint, doc and some more polishing
* Addressing review comments and further simplifying the code
* Address more review comments
* Use explicit temp storage for mapping to avoid E0716 in Rust 1.70
* Allow checking 2Q non-control flow instructions
* Fully port InverseCancellation to Rust
This commit builds off of #12959 and the other data model in Rust
infrastructure and migrates the InverseCancellation pass to
operate fully in Rust. The full path of the transpiler pass now never
leaves Rust until it has finished modifying the DAGCircuit. There is
still some python interaction necessary to handle parts of the data
model that are still in Python, mainly for handling parameter
expressions. But otherwise the entirety of the pass
operates in rust now.
This is just a first pass at the migration here, it moves the pass to
use loops in rust. The next steps here are to look at operating
the pass in parallel. There is no data dependency between the
optimizations being done for different inverse gates/pairs so we should
be able to the throughput of the pass by leveraging multithreading to
handle each inverse option in parallel. This commit does not attempt
this though, because of the Python dependency and also the data
structures around gates and the dag aren't really setup for
multithreading yet and there likely will need to be some work to
support that.
Fixes#12271
Part of #12208
* Remove temporary variable for chunk empty check
* Destructure gate pairs
* Rework short circuit logic
* Fully port CheckMap to Rust
This commit migrates the entirety of the CheckMap analysis pass to Rust.
The pass operates solely in the rust domain and returns an
`Option<(String, [u32; 2])>` to Python which is used to set the two
property set fields appropriately. All the analysis of the dag is done
in Rust. There is still Python interaction required though because
control flow operations are only defined in Python. However the
interaction is minimal and only to get the circuits for control flow
blocks and converting them into DAGs (at least until #13001 is complete).
This commit is based on top of #12959 and will need to be rebased after
that merges.
Closes#12251
Part of #12208
* Use a Vec<Qubit> for wire_map instead of a HashMap
This commit switches to using a Vec<Qubit> for the internal wire_map
used to map control flow qubits. A HashMap was originally used because
in Python a dictionary is used. However, in the rust domain the inner
qubits are contiguous integers starting from 0 so a Vec can be used for
better performance in the case we have control flow.
* Update crates/accelerate/src/check_map.rs
Co-authored-by: Raynel Sanchez <87539502+raynelfss@users.noreply.github.com>
---------
Co-authored-by: Raynel Sanchez <87539502+raynelfss@users.noreply.github.com>
* Initial: Add add_from_iter method to DAGCircuit
- Introduce a method that adds a chain of `PackedInstruction` continuously avoiding the re-linking of each bit's output-node until the very end of the iterator.
- TODO: Add handling of vars
- Add header for a `from_iter` function that will create a `DAGCircuit` based on a chain of `PackedInstruction`.
* Fix: leverage new methods in layers
- Fix incorrect re-insertion of last_node.
* Fix: Keep track of Vars for add_from_iter
- Remove `from_iter`
* Fix: Incorrect modification of last nodes in `add_from_iter`.
- Use `entry` api to either modify or insert a value if missing.
* Fix: Cycling edges in when adding vars.
- A bug that adds duplicate edges to vars has been temporarily fixed. However, the root of this problem hasn't been found yet. A proper fix is pending. For now skip those instances.
* Fix: Remove set collecting all nodes to be connected.
- A set collecting all the new nodes to connect with a new node was preventing additional wires to connect to subsequent nodes.
* Fix: Adapt to #13033
* Refactor: `add_from_iter` is now called `extend` to stick with `Rust` nomenclature.
* Fix docstring
- Caught by @ElePT
Co-authored-by: Elena Peña Tapia <57907331+ElePT@users.noreply.github.com>
* Fix: Remove duplicate vars check
* Fix: Corrections from code review.
- Use Entry API to modify last nodes in the var.
- Build new_nodes with an allocated vec.
- Add comment explaining the removal of the edge between the output node and its predecessor.
* Fix: Improper use of `Entry API`.
- Use `or_insert_with` instead of `or_insert` to perform actions before inserting a value.
---------
Co-authored-by: Elena Peña Tapia <57907331+ElePT@users.noreply.github.com>
The recently released Rust 1.81.0 introduced some new on by default
clippy rules and these rules are flagging issues in the rust code in the
library. While we use Rust 1.70 for clippy in CI and these won't cause
failures until we raise our MSRV to >= 1.81.0 these clippy
warnings/failures are still good to fix as the either make the code more
consise and/or efficient. This commit fixes these issues identified by
clippy.
* Fully port FilterOpNodes to Rust
This commit ports the FilterOpNodes pass to rust. This pass is
exceedingly simple and just runs a filter function over all the op
nodes and removes nodes that match the filter. However, the API for
the class exposes that filter function interface as a user provided
Python callable. So for the current pass we need to retain that python
callback. This limits the absolute performance of this pass because
we're bottlenecked by calling python.
Looking to the future, this commit adds a rust native method to
DAGCircuit to perform this filtering with a rust predicate FnMut. It
isn't leveraged by the python implementation because of layer mismatch
for the efficient rust interface and Python working with `DAGOpNode`
objects. A function using that interface is added to filter labeled
nodes. In the preset pass manager we only use FilterOpNodes to remove
nodes with a specific label (which is used to identify temporary
barriers created by qiskit). In a follow up we should consider
leveraging this new function to build a new pass specifically for
this use case.
Fixes#12263
Part of #12208
* Make filter_op_nodes() infallible
The filter_op_nodes() method originally returned a Result<()> to handle
a predicate that was fallible. This was because the original intent for
the method was to use it with Python callbacks in the predicate. But
because of differences between the rust API and the Python API this
wasn't feasible as was originally planned. So this Result<()> return
wasn't used anymore. This commit reworks it to make the
filter_op_nodes() infallible and the predicate a user provides also only
returns `bool` and not `Result<bool>`.
* Rename filter_labelled_op to filter_labeled_op
* add rust code for remove_diagonal_gates_before_measure
* replace python code by rust code for remove_diagonal_gates_brefore_measure
* make some dag_circuit functions public
* fix some of the tests failures
* fix another failing test, add more diagonal gates
* add tests for added diagonal gates: p, cp, cs, csdg
* make the lists of diagonal gates into static arrays
* formatting
* change nodes_to_remove type to Vec
* remove the HashSet following review
* add CCZ diagonal gate
* add tests for CCZ diagonal gate
* add release notes
* fix lint
* Adjust graphviz options for better layout with plot_gate_map()
This commit fixes an issue with the visualizations of some
backends/coupling maps. The default neato settings worked well in most
cases but for some graphs it was not generating a good layout. This
tweaks the settings a bit so that layouts work with more classes of
graphs.
* Don't set graph attributes if qubit_coordinates is set
* adjust sizes
* planar branching
* support for backendv1
* remove the formula
* new ref images
---------
Co-authored-by: Luciano Bello <bel@zurich.ibm.com>
Co-authored-by: Elena Peña Tapia <57907331+ElePT@users.noreply.github.com>
* init
* up
* lint
* .
* up
* before cache
* with cache
* correct
* cleaned up
* lint reno
* Update Cargo.lock
* .
* up
* .
* revert op
* .
* .
* .
* .
* Delete Cargo.lock
* .
* corrected string comparison
* removed Operator class from operation.rs
* .
* Apply suggestions from code review
Co-authored-by: Raynel Sanchez <87539502+raynelfss@users.noreply.github.com>
* comments from code review
* Port DAGCircuit to Rust
This commit migrates the entirety of the `DAGCircuit` class to Rust. It
fully replaces the Python version of the class. The primary advantage
of this migration is moving from a Python space rustworkx directed graph
representation to a Rust space petgraph (the upstream library for
rustworkx) directed graph. Moving the graph data structure to rust
enables us to directly interact with the DAG directly from transpiler
passes in Rust in the future. This will enable a significant speed-up in
those transpiler passes. Additionally, this should also improve the
memory footprint as the DAGCircuit no longer stores `DAGNode`
instances, and instead stores a lighter enum NodeType, which simply
contains a `PackedInstruction` or the wire objects directly.
Internally, the new Rust-based `DAGCircuit` uses a `petgraph::StableGraph`
with node weights of type `NodeType` and edge weights of type `Wire`. The
NodeType enum contains variants for `QubitIn`, `QubitOut`, `ClbitIn`,
`ClbitOut`, and `Operation`, which should save us from all of the
`isinstance` checking previously needed when working with `DAGNode` Python
instances. The `Wire` enum contains variants `Qubit`, `Clbit`, and `Var`.
As the full Qiskit data model is not rust-native at this point while
all the class code in the `DAGCircuit` exists in Rust now, there are
still sections that rely on Python or actively run Python code via Rust
to function. These typically involve anything that uses `condition`,
control flow, classical vars, calibrations, bit/register manipulation,
etc. In the future as we either migrate this functionality to Rust or
deprecate and remove it this can be updated in place to avoid the use
of Python.
API access from Python-space remains in terms of `DAGNode` instances to
maintain API compatibility with the Python implementation. However,
internally, we convert to and deal in terms of NodeType. When the user
requests a particular node via lookup or iteration, we inflate an ephemeral
`DAGNode` based on the internal `NodeType` and give them that. This is very
similar to what was done in #10827 when porting CircuitData to Rust.
As part of this porting there are a few small differences to keep in
mind with the new Rust implementation of DAGCircuit. The first is that
the topological ordering is slightly different with the new DAGCircuit.
Previously, the Python version of `DAGCircuit` using a lexicographical
topological sort key which was basically `"0,1,0,2"` where the first
`0,1` are qargs on qubit indices `0,1` for nodes and `0,2` are cargs
on clbit indices `0,2`. However, the sort key has now changed to be
`(&[Qubit(0), Qubit(1)], &[Clbit(0), Clbit(2)])` in rust in this case
which for the most part should behave identically, but there are some
edge cases that will appear where the sort order is different. It will
always be a valid topological ordering as the lexicographical key is
used as a tie breaker when generating a topological sort. But if you're
relaying on the exact same sort order there will be differences after
this PR. The second is that a lot of undocumented functionality in the
DAGCircuit which previously worked because of Python's implicit support
for interacting with data structures is no longer functional. For
example, previously the `DAGCircuit.qubits` list could be set directly
(as the circuit visualizers previously did), but this was never
documented as supported (and would corrupt the DAGCircuit). Any
functionality like this we'd have to explicit include in the Rust
implementation and as they were not included in the documented public
API this PR opted to remove the vast majority of this type of
functionality.
The last related thing might require future work to mitigate is that
this PR breaks the linkage between `DAGNode` and the underlying
`DAGCirucit` object. In the Python implementation the `DAGNode` objects
were stored directly in the `DAGCircuit` and when an API method returned
a `DAGNode` from the DAG it was a shared reference to the underlying
object in the `DAGCircuit`. This meant if you mutated the `DAGNode` it
would be reflected in the `DAGCircuit`. This was not always a sound
usage of the API as the `DAGCircuit` was implicitly caching many
attributes of the DAG and you should always be using the `DAGCircuit`
API to mutate any nodes to prevent any corruption of the `DAGCircuit`.
However, now as the underlying data store for nodes in the DAG are
no longer the python space objects returned by `DAGCircuit` methods
mutating a `DAGNode` will not make any change in the underlying
`DAGCircuit`. This can come as quite the surprise at first, especially
if you were relying on this side effect, even if it was unsound.
It's also worth noting that 2 large pieces of functionality from
rustworkx are included in this PR. These are the new files
`rustworkx_core_vnext` and `dot_utils` which are rustworkx's VF2
implementation and its dot file generation. As there was not a rust
interface exposed for this functionality from rustworkx-core there was
no way to use these functions in rustworkx. Until these interfaces
added to rustworkx-core in future releases we'll have to keep these
local copies. The vf2 implementation is in progress in
Qiskit/rustworkx#1235, but `dot_utils` might make sense to keep around
longer term as it is slightly modified from the upstream rustworkx
implementation to directly interface with `DAGCircuit` instead of a
generic graph.
Co-authored-by: Matthew Treinish <mtreinish@kortar.org>
Co-authored-by: Raynel Sanchez <87539502+raynelfss@users.noreply.github.com>
Co-authored-by: Elena Peña Tapia <57907331+ElePT@users.noreply.github.com>
Co-authored-by: Alexander Ivrii <alexi@il.ibm.com>
Co-authored-by: Eli Arbel <46826214+eliarbel@users.noreply.github.com>
Co-authored-by: John Lapeyre <jlapeyre@users.noreply.github.com>
Co-authored-by: Jake Lishman <jake.lishman@ibm.com>
* Update visual mpl circuit drawer references
Right now there is a bug in the matplotlib circuit visualizer likely
caused by the new `__eq__` implementation for `DAGOpNode` that didn't
exist before were some gates are missing from the visualization. In the
interest of unblocking this PR this commit updates the references for
these cases temporarily until this issue is fixed.
* Ensure DAGNode.sort_key is always a string
Previously the sort_key attribute of the Python space DAGCircuit was
incorrectly being set to `None` for rust generated node objects. This
was done as for the default path the sort key is determined from the
rust domain's representation of qubits and there is no analogous data in
the Python object. However, this was indavertandly a breaking API change
as sort_key is expected to always be a string. This commit adds a
default string to use for all node types so that we always have a
reasonable value that matches the typing of the class. A future step is
likely to add back the `dag` kwarg to the node types and generate the
string on the fly from the rust space data.
* Make Python argument first in Param::eq and Param::is_close
The standard function signature convention for functions that take a
`py: Python` argument is to make the Python argument the first (or
second after `&self`). The `Param::eq` and `Param::is_close` methods
were not following this convention and had `py` as a later argument in
the signature. This commit corrects the oversight.
* Fix merge conflict with #12943
With the recent merge with main we pulled in #12943 which conflicted
with the rust space API changes made in this PR branch. This commit
updates the usage to conform with the new interface introduced in this
PR.
* Add release notes and test for invalid args on apply methods
This commit adds several release notes to document this change. This
includes a feature note to describe the high level change and the user
facing benefit (mainly reduced memory consumption for DAGCircuits),
two upgrade notes to document the differences with shared references
caused by the new data structure, and a fix note documenting the fix
for how qargs and cargs are handled on `.apply_operation_back()` and
`.apply_operation_front()`. Along with the fix note a new unit test is
added to serve as a regression test so that we don't accidentally allow
adding cargs as qargs and vice versa in the future.
* Restore `inplace` argument functionality for substitute_node()
This commit restores the functionality of the `inplace` argument for
`substitute_node()` and restores the tests validating the object
identity when using the flag. This flag was originally excluded from
the implementation because the Rust representation of the dag is not
a shared reference with Python space and the flag doesn't really mean
the same thing as there is always a second copy of the data for Python
space now. The implementation here is cheating slighty as we're passed
in the DAG node by reference it relies on that reference to update the
input node at the same time we update the dag. Unlike the previous
Python implementation where we were updating the node in place and the
`inplace` argument was slightly faster because everything was done by
reference. The rust space data is still a compressed copy of the data
we return to Python so the `inplace` flag will be slightly more
inefficient as we need to copy to update the Python space representation
in addition to the rust version.
* Revert needless dict() cast on metadata in dag_to_circuit()
This commit removes an unecessary `dict()` cast on the `dag.metadata`
when setting it on `QuantumCircuit.metadata` in
`qiskit.converters.dag_to_circuit()`. This slipped in at some point
during the development of this PR and it's not clear why, but it isn't
needed so this removes it.
* Add code comment for DAGOpNode.__eq__ parameter checking
This commit adds a small inline code comment to make it clear why we
skip parameter comparisons in DAGOpNode.__eq__ for python ops. It might
not be clear why the value is hard coded to `true` in this case, as this
check is done via Python so we don't need to duplicate it in rust space.
* Raise a ValueError on DAGNode creation with invalid index
This commit adds error checking to the DAGNode constructor to raise a
PyValueError if the input index is not valid (any index < -1).
Previously this would have panicked instead of raising a user catchable
error.
* Use macro argument to set python getter/setter name
This commit updates the function names for `get__node_id` and
`set__node_id` method to use a name that clippy is happy with and
leverage the pyo3 macros to set the python space name correctly instead
of using the implicit naming rules.
* Remove Ord and PartialOrd derives from interner::Index
The Ord and PartialOrd traits were originally added to the Index struct
so they could be used for the sort key in lexicographical topological
sorting. However, that approach was abandonded during the development of
this PR and instead the expanded Qubit and Clbit indices were used
instead. This left the ordering traits as unnecessary on Index and
potentially misleading. This commit just opts to remove them as they're
not needed anymore.
* Fix missing nodes in matplotlib drawer.
Previously, the change in equality for DAGNodes was causing nodes
to clobber eachother in the matplotlib drawer's tracking data
structures when used as keys to maps.
To fix this, we ensure that all nodes have a unique ID across
layers before constructing the matplotlib drawer. They actually
of course _do_ in the original DAG, but we don't really care
what the original IDs are, so we just make them up.
Writing to _node_id on a DAGNode may seem odd, but it exists
in the old Python API (prior to being ported to Rust) and
doesn't actually mutate the DAG at all since DAGNodes are
ephemeral.
* Revert "Update visual mpl circuit drawer references"
With the previous commit the bug in the matplotlib drawer causing the
images to diverge should be fixed. This commit reverts the change to the
reference images as there should be no difference now.
This reverts commit 1e4e6f3862.
* Update visual mpl circuit drawer references for control flow circuits
The earlier commit that "fixed" the drawers corrected the visualization
to match expectations in most cases. However after restoring the
references to what's on main several comparison tests with control flow
in the circuit were still failing. The failure mode looks similar to the
other cases, but across control flow blocks instead of at the circuit
level. This commit temporarily updates the references of these to the
state of what is generated currently to unblock CI. If/when we have a
fix this commit can be reverted.
* Apply suggestions from code review
Co-authored-by: Raynel Sanchez <87539502+raynelfss@users.noreply.github.com>
* code review
* Fix edge cases in DAGOpNode.__eq__
This commit fixes a couple of edge cases in DAGOpNode.__eq__ method
around the python interaction for the method. The first is that in
the case where we had python object parameter types for the gates we
weren't comparing them at all. This is fixed so we use python object
equality for the params in this case. Then we were dropping the error
handling in the case of using python for equality, this fixes it to
return the error to users if the equality check fails. Finally a comment
is added to explain the expected use case for `DAGOpNode.__eq__` and why
parameter checking is more strict than elsewhere.
* Remove Param::add() for global phase addition
This commit removes the Param::add() method and instead adds a local
private function to the `dag_circuit` module for doing global phase
addition. Previously the `Param::add()` method was used solely for
adding global phase in `DAGCircuit` and it took some shortcuts knowing
that context. This made the method implementation ill suited as a
general implementation.
* More complete fix for matplotlib drawer.
* Revert "Update visual mpl circuit drawer references for control flow circuits"
This reverts commit 9a6f9536a3.
* Unify rayon versions in workspace
* Remove unused _GLOBAL_NID.
* Use global monotonic ID counter for ids in drawer
The fundamental issue with matplotlib visualizations of control flow is
that locally in the control flow block the nodes look the same but are
stored in an outer circuit dictionary. If the gates are the same and on
the same qubits and happen to have the same node id inside the different
control flow blocks the drawer would think it's already drawn the node
and skip it incorrectly. The previous fix for this didn't go far enough
because it wasn't accounting for the recursive execution of the drawer
for inner blocks (it also didn't account for LayerSpoolers of the same
length).
* Remove unused BitData iterator stuff.
* Fully port Optimize1qGatesDecomposition to Rust
This commit builds off of #12550 and the other data model in Rust
infrastructure and migrates the Optimize1qGatesDecomposition pass to
operate fully in Rust. The full path of the transpiler pass now never
leaves rust until it has finished modifying the DAGCircuit. There is
still some python interaction necessary to handle parts of the data
model that are still in Python, mainly calibrations and parameter
expressions (for global phase). But otherwise the entirety of the pass
operates in rust now.
This is just a first pass at the migration here, it moves the pass to be
a single for loop in rust. The next steps here are to look at operating
the pass in parallel. There is no data dependency between the
optimizations being done by the pass so we should be able to the
throughput of the pass by leveraging multithreading to handle each run
in parallel. This commit does not attempt this though, because of the
Python dependency and also the data structures around gates and the
dag aren't really setup for multithreading yet and there likely will
need to be some work to support that (this pass is a good candidate to
work through the bugs on that).
Part of #12208
* remove with_gil in favor of passing python tokens as params
* Apply suggestions from code review
Co-authored-by: Raynel Sanchez <87539502+raynelfss@users.noreply.github.com>
* fmt
* python serialization
* deprecation
* Update commutation_checker.py
* heh
* init
* let Pytuple collect
* lint
* First set of comments
- use Qubit/Clbit
- more info on unsafe
- update reno
- use LazySet less
- use OperationRef, avoid CircuitInstruction creation
* Second part
- clippy
- no BigInt
- more comments
* Matrix speed & fix string sort
-- could not use op.name() directly since sorted differently than Python, hence it's back to BigInt
* have the Python implementation use Rust
* lint & tools
* remove unsafe blocks
* One more try to avoid segfaulty windows
-- if that doesn't work maybe revert the change the the Py CommChecker uses Rust
* Original version
Co-authored-by: Sebastian Brandhofer <148463728+sbrandhsn@users.noreply.github.com>
* Sync with updated CommutationChecker
todo: shouldn't make the qubits interner public
* Debug: disable cache
trying to figure out why the windows CI fails (after being unable to locally reproduce we're using CI with a reduced set of tests)
* ... second try
* Update crates/accelerate/src/commutation_checker.rs
Co-authored-by: Raynel Sanchez <87539502+raynelfss@users.noreply.github.com>
* Restore azure config
* Remove unused import
* Revert "Debug: disable cache"
This reverts commit c564b806c9.
* Don't overallocate cache
We were allocating a the cache hashmap with a capacity for max cache
size entries every time we instantiated a new CommutationChecker. The
max cache size is 1 million. This meant we were allocating 162MB
everytime CommutationChecker.__new__ was called, which includes each
time we instantiate it manually (which happens once on import), the
CommutationAnalysis pass gets instantiated (twice per preset pass
manager created with level 2 or 3), or a commutation checker instance is
pickle deserialized. This ends up causing a fairly large memory
regression and is the source of the CI failures on windows.
Co-authored-by: Jake Lishman <jake.lishman@ibm.com>
* Cleanup parameter key type to handle edge conditions better
This commit cleans up the ParameterKey type and usage to make it handle
edge conditions better. The first is that the type just doesn't do the
right thing for NaN, -0, or the infinities. Canonicalization is added
for hash on -0 and the only constructor of the newtype adds a runtime
guard against NaN and inifinity (positive or negative) to avoid that
issue. The approach only makes sense as the cache is really there to
guard us against unnecessary re-computing when we reiterate over the
circuit > 1 time and nothing has changed for gates. Otherwise comparing
floats like done in this PR does would not be a sound or an effective
approach.
* Remove unnecessary cache hit rate tracking
* Undo test assertion changes
* Undo unrelated test changes
* Undo pending deprecation and unify commutation classes
This commit removes the pending deprecation decorator from the python
class definition as the Python class just internally is using the rust
implementation now. This also removes directly using the rust
implementation for the standard commutation library global as using the
python class is exactly the same now.
We can revisit if there is anything we want to deprecate and remove in
2.0 in a follow up PR. Personally, I think the cache management methods
are all we really want to remove as the cache should be an internal
implementation detail and not part of the public interface.
* Undo gha config changes
* Make serialization explicit
This commit makes the pickling of cache entries explicit. Previously it
was relying on conversion traits which hid some of the complexity but
this uses a pair of conversion functions instead.
* Remove stray SAFETY comment
* Remove ddt usage from the tests
Now that the python commutation checker and the rust commutation checker
are the same thing the ddt parameterization of the commutation checker
tests was unecessary duplication. This commit removes the ddt usage to
restore having a single run of all the tests.
* Update release note
* Fix CommutationChecker class import
* Remove invalid test assertion for no longer public attribute
* Ray's review comments
Co-authored-by: Raynel Sanchez <raynelfss@hotmail.com>
* Handle ``atol/rtol``, more error propagation
* update to latest changes in commchecker
* fix merge conflict remnants
* re-use expensive quantities
such as the relative placement and the parameter hash
* add missing header
* gentler error handling
* review comments & more docs
* Use vec over IndexSet + clippy
- vec<vec> is slightly faster than vec<indexset>
- add custom types to satisfies clippy's complex type complaint
- don't handle Clbit/Var
* Simplify python class construction
Since this PR was first written the split between the python side and
rust side of the CommutationChecker class has changed so that there are
no longer separate classes anymore. The implementations are unified and
the python space class just wraps an inner rust object. However, the
construction of the CommutationAnalysis pass was still written assuming
there was the possibility to get either a rust or Python object. This
commit fixes this and the type change on the `comm_checker` attribute by
removing the unnecessary logic.
---------
Co-authored-by: Raynel Sanchez <87539502+raynelfss@users.noreply.github.com>
Co-authored-by: Kevin Hartman <kevin@hart.mn>
Co-authored-by: Matthew Treinish <mtreinish@kortar.org>
Co-authored-by: Elena Peña Tapia <57907331+ElePT@users.noreply.github.com>
Co-authored-by: Alexander Ivrii <alexi@il.ibm.com>
Co-authored-by: Eli Arbel <46826214+eliarbel@users.noreply.github.com>
Co-authored-by: John Lapeyre <jlapeyre@users.noreply.github.com>
Co-authored-by: Jake Lishman <jake.lishman@ibm.com>
Co-authored-by: Julien Gacon <jules.gacon@googlemail.com>
Co-authored-by: Raynel Sanchez <raynelfss@hotmail.com>
* Initial: implement fixed capacity constructor for DAGCircuit
- Implement `DAGCircuit` with `with_capacity` to create an initially allocated instance, for rust only.
- Implement `with_capacity` for `BitData` and `IndexInterner` that creates an initally allocated instance of each type.
- Other small tweaks and fixes.
- Add num_edges optional argument. If known, use `num_edges` argument to create a `DAGCircuit` with the exact number of edges.
- Leverage usage of new method in `copy_empty_like`.
- Use `..Default::default()` for `op_names` and `calibrations` fields in `DAGCircuit::with_capacity()`
* Fix: Adapt to #13033
* Fix: Re-arrange the function arguments as per @mtreinish's review.
- The order now follows: qubits, clbits, vars, num_ops, edges. As per [Matthew's comment](https://github.com/Qiskit/qiskit/pull/12975#discussion_r1742426093).
The default value for `Instruction.unit` is `"dt"`. Previously, the
`OperationFromPython` extraction logic would only suppress allocation of
the extra instruction attributes if all the contained fields were
`None`, but `None` is not actually a valid value of `Instruction.unit`
(which must be a string). This meant that `OperationFromPython` would
always allocate and store extra attributes, even for the default cases.
This did not affect standard gates appended using their corresponding
`QuantumCircuit` methods (since no Python-space extraction is
performed in that case), but did affect standard calls to `append`, or
anything else that entered from Python space.
This drastically reduces the memory usage of circuits built by
`append`-like methods. Ignoring the inefficiency factor of the
heap-allocation implementation, this saves 66 bytes plus
small-allocation overhead for 2-byte heap allocations (another 14 bytes
on macOS, but will vary depending on the allocator) per standard
instruction, which is on the order of 40% memory-usage reduction.
Matthew Treinish