Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Move preset pass manager transpiler passes to rust #12208

Open
17 of 39 tasks
mtreinish opened this issue Apr 18, 2024 · 0 comments
Open
17 of 39 tasks

Move preset pass manager transpiler passes to rust #12208

mtreinish opened this issue Apr 18, 2024 · 0 comments
Labels
performance priority: high Rust This PR or issue is related to Rust code in the repository type: epic A theme of work that contain sub-tasks type: feature request New feature or request
Milestone

Comments

@mtreinish
Copy link
Member

mtreinish commented Apr 18, 2024

What should we add?

As part of our ongoing effort to migrate more of the rust code for better runtime performance and efficiency we are looking to migrate all the transpiler passes in the preset pass managers to be written in rust. We will still primarily use the python interface for passes as this is a critical part of the transpiler, but the default passes we use should execute solely in rust space and the python portion should just control the dispatch of the rust function.

This is blocked on #11721 and in many cases on #12205 too

The current list of passes involved for this are tracked here:

Tasks

Preview Give feedback
  1. Rust mod: transpiler performance priority: high type: feature request
    ElePT
  2. Rust mod: transpiler performance priority: high
    alexanderivrii
  3. 4 of 4
    Rust mod: transpiler performance priority: high
    raynelfss
  4. Rust mod: transpiler performance
  5. Rust mod: transpiler performance
  6. Rust mod: transpiler performance
    henryzou50 mtreinish
  7. Rust mod: transpiler performance
    mtreinish
  8. Rust mod: transpiler performance
    ElePT
  9. Rust mod: transpiler performance
    eliarbel
  10. Rust mod: transpiler performance
  11. Rust mod: transpiler performance
    ShellyGarion
  12. Rust mod: transpiler performance
    alexanderivrii
  13. Rust mod: transpiler performance
    henryzou50
  14. Rust mod: transpiler performance
    eliarbel
  15. Rust mod: transpiler performance
  16. Rust mod: transpiler performance
  17. Rust mod: transpiler performance
  18. Rust mod: transpiler performance
  19. Rust mod: transpiler performance
  20. Rust mod: transpiler performance
  21. Rust mod: transpiler performance
    mtreinish
  22. Rust mod: transpiler performance
  23. Rust mod: transpiler performance
  24. Rust mod: transpiler performance
  25. Rust mod: transpiler performance
  26. Rust mod: transpiler performance
  27. Rust mod: transpiler performance
  28. Rust mod: transpiler performance priority: high
    sbrandhsn
  29. Rust mod: transpiler performance
    mtreinish
  30. Rust mod: transpiler performance
    kevinhartman
  31. Rust mod: transpiler performance
    kevinhartman
  32. Rust mod: transpiler performance
  33. Rust mod: transpiler performance
    kevinhartman
  34. Rust mod: transpiler performance
  35. Rust mod: transpiler performance
    jakelishman
  36. Rust mod: transpiler performance
  37. Rust mod: transpiler performance
  38. Rust mod: transpiler performance

I included passes that are primarily written in rust because there is still some python side interaction because we don't have #12205 and #11721 yet. (this is current as of 04-18-2024, we should update this if we make changes to the preset pass managers)

@mtreinish mtreinish added type: feature request New feature or request type: epic A theme of work that contain sub-tasks Rust This PR or issue is related to Rust code in the repository priority: high labels Apr 18, 2024
mtreinish added a commit to mtreinish/qiskit-core that referenced this issue Aug 14, 2024
This commit builds off of Qiskit#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 Qiskit#12208
mtreinish added a commit to mtreinish/qiskit-core that referenced this issue Aug 14, 2024
This commit builds off of Qiskit#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 Qiskit#12208
mtreinish added a commit to mtreinish/qiskit-core that referenced this issue Aug 15, 2024
This commit builds off of Qiskit#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 Qiskit#12208
mtreinish added a commit to mtreinish/qiskit-core that referenced this issue Aug 15, 2024
This commit builds off of Qiskit#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 Qiskit#12208
mtreinish added a commit to mtreinish/qiskit-core that referenced this issue Aug 16, 2024
This commit builds off of Qiskit#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 Qiskit#12208
mtreinish added a commit to mtreinish/qiskit-core that referenced this issue Aug 17, 2024
This commit builds off of Qiskit#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 Qiskit#12208
mtreinish added a commit to mtreinish/qiskit-core that referenced this issue Aug 17, 2024
This commit builds off of Qiskit#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 Qiskit#12208
mtreinish added a commit to mtreinish/qiskit-core that referenced this issue Aug 20, 2024
This commit builds off of Qiskit#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 Qiskit#12208
mtreinish added a commit to mtreinish/qiskit-core that referenced this issue Aug 21, 2024
This commit builds off of Qiskit#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 Qiskit#12271
Part of Qiskit#12208
mtreinish added a commit to mtreinish/qiskit-core that referenced this issue Aug 22, 2024
This commit builds off of Qiskit#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 Qiskit#12208
mtreinish added a commit to mtreinish/qiskit-core that referenced this issue Aug 22, 2024
This commit builds off of Qiskit#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 Qiskit#12271
Part of Qiskit#12208
mtreinish added a commit to mtreinish/qiskit-core that referenced this issue Aug 23, 2024
This commit builds off of Qiskit#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 Qiskit#12208
mtreinish added a commit to mtreinish/qiskit-core that referenced this issue Aug 23, 2024
This commit builds off of Qiskit#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 Qiskit#12208
mtreinish added a commit to mtreinish/qiskit-core that referenced this issue Aug 24, 2024
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 Qiskit#13001 is complete).

This commit is based on top of Qiskit#12959 and will need to be rebased after
that merges.

Closes Qiskit#12251
Part of Qiskit#12208
mtreinish added a commit to mtreinish/qiskit-core that referenced this issue Aug 25, 2024
This commit builds off of Qiskit#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 Qiskit#12271
Part of Qiskit#12208
mtreinish added a commit to mtreinish/qiskit-core that referenced this issue Aug 25, 2024
This commit builds off of Qiskit#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 Qiskit#12208
mtreinish added a commit to mtreinish/qiskit-core that referenced this issue Aug 28, 2024
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 Qiskit#12263
Part of Qiskit#12208
mtreinish added a commit to mtreinish/qiskit-core that referenced this issue Aug 28, 2024
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 Qiskit#12263
Part of Qiskit#12208
github-merge-queue bot pushed a commit that referenced this issue Aug 30, 2024
* 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

* Tweak control_flow_op_nodes() method to avoid dag traversal when not necessary

* Store target basis set without heap allocation

Since we only are storing 12 enum fields (which are a single byte) using
any heap allocated collection is completely overkill and will have more
overhead that storing a statically sized array for all 12 variants. This
commit adds a new struct that wraps a `[bool; 12]` to track which
basis are supported and an API for tracking this. This simplifies the
tracking of which qubit supports which EulerBasis, it also means other
internal users of the 1q decomposition have a simplified API for working
with the euler basis.

* Remove From trait for Qubit->PhysicalQubit conversion

* Fix merge conflict

* Use new DAGCircuit::has_control_flow() for control_flow_op_nodes() pymethod

* Move _basis_gates set creation to __init__

* Update releasenotes/notes/optimize-1q-gates-decomposition-ce111961b6782ee0.yaml

Co-authored-by: Elena Peña Tapia <[email protected]>

---------

Co-authored-by: Elena Peña Tapia <[email protected]>
mtreinish added a commit to mtreinish/qiskit-core that referenced this issue Aug 30, 2024
This commit builds off of Qiskit#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 Qiskit#12271
Part of Qiskit#12208
mtreinish added a commit to mtreinish/qiskit-core that referenced this issue Aug 30, 2024
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 Qiskit#13001 is complete).

This commit is based on top of Qiskit#12959 and will need to be rebased after
that merges.

Closes Qiskit#12251
Part of Qiskit#12208
mtreinish added a commit to mtreinish/qiskit-core that referenced this issue Aug 30, 2024
This commit builds off of Qiskit#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 Qiskit#12208
github-merge-queue bot pushed a commit that referenced this issue Sep 4, 2024
* 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 <[email protected]>

* 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 <[email protected]>
Co-authored-by: Raynel Sanchez <[email protected]>
Co-authored-by: Elena Peña Tapia <[email protected]>
Co-authored-by: Alexander Ivrii <[email protected]>
Co-authored-by: Eli Arbel <[email protected]>
Co-authored-by: John Lapeyre <[email protected]>
Co-authored-by: Jake Lishman <[email protected]>

* 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 1e4e6f3.

* 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 <[email protected]>

* 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 9a6f953.

* 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 <[email protected]>

* 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 <[email protected]>

* 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 <[email protected]>

* Restore azure config

* Remove unused import

* Revert "Debug: disable cache"

This reverts commit c564b80.

* 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 <[email protected]>

* 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 <[email protected]>

* 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 <[email protected]>
Co-authored-by: Kevin Hartman <[email protected]>
Co-authored-by: Matthew Treinish <[email protected]>
Co-authored-by: Elena Peña Tapia <[email protected]>
Co-authored-by: Alexander Ivrii <[email protected]>
Co-authored-by: Eli Arbel <[email protected]>
Co-authored-by: John Lapeyre <[email protected]>
Co-authored-by: Jake Lishman <[email protected]>
Co-authored-by: Julien Gacon <[email protected]>
Co-authored-by: Raynel Sanchez <[email protected]>
github-merge-queue bot pushed a commit that referenced this issue Sep 5, 2024
* 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
github-merge-queue bot pushed a commit that referenced this issue Sep 6, 2024
* 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 <[email protected]>

---------

Co-authored-by: Raynel Sanchez <[email protected]>
github-merge-queue bot pushed a commit that referenced this issue Sep 6, 2024
* 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
github-merge-queue bot pushed a commit that referenced this issue Sep 9, 2024
* 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
@mtreinish mtreinish added this to the 1.3.0 milestone Oct 10, 2024
@raynelfss raynelfss modified the milestones: 1.3.0, 2.0.0 Nov 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
performance priority: high Rust This PR or issue is related to Rust code in the repository type: epic A theme of work that contain sub-tasks type: feature request New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants