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.

Sign up for GitHub

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

Jump to bottom

Improve 1q decomposition pass with multiple matching basis #5431

Merged
merged 16 commits into from
Dec 11, 2020
Merged

Improve 1q decomposition pass with multiple matching basis #5431

merged 16 commits into from
Dec 11, 2020

Conversation

mtreinish
Copy link
Member

Summary

This commit makes an improvement to the Optimize1qGatesDecomposition
pass to improve the output from it in two scenarios. The first scenario
is if there is an over complete basis where there are multiple choices
for decomposition basis. Previously in such scenarios only the first
matching subset would be used. This improves that by finding runs in all
matching decomposition basis to simplify any runs in those gate sets.
The second improvement made here is that the decomposition is only used
if it results in a decrease in depth. There were scenarios where the
general decomposition of a run would result in more gates than the
input to the OneQubitEulerDecomposer (typically 3 gates vs 2 input). In
those cases we shouldn't use the decomposition and instead rely on the
original run because we're adding gates which is the opposit of the
expected behavior.

Details and comments

@mtreinish mtreinish requested a review from a team as a code owner November 24, 2020 19:37
@mtreinish mtreinish added this to the 0.17 milestone Nov 24, 2020
@ikkoham ikkoham mentioned this pull request Nov 25, 2020
Open
@mtreinish mtreinish requested a review from kdk November 25, 2020 18:43
mtreinish added a commit to mtreinish/retworkx that referenced this pull request Nov 25, 2020
@mtreinish
Add collect_runs() function
3831c65 
This commit adds a new algorithms function, collect_runs(), which is
used to find a list of runs in a PyDiGraph object that match a specified
condition. A run in this context is any linear path inside the graph.
The condition is specified using a python function that will be passed a
node's data payload/weight object and will return whether it matches the
condition or not. The intended use case for this function is with
qiskit-terra's PyDiGraph method collect_runs() which is implementing
this algorithm currently in Python on top of PyDiGraph (it will also be
used by a future method collect_1q_runs which is being added in
Qiskit/qiskit#5431).
This was referenced Nov 25, 2020
Merged
Closed
mtreinish added a commit to mtreinish/retworkx that referenced this pull request Nov 29, 2020
@mtreinish
Add collect_runs() function
d80bda7 
This commit adds a new algorithms function, collect_runs(), which is
used to find a list of runs in a PyDiGraph object that match a specified
condition. A run in this context is any linear path inside the graph.
The condition is specified using a python function that will be passed a
node's data payload/weight object and will return whether it matches the
condition or not. The intended use case for this function is with
qiskit-terra's PyDiGraph method collect_runs() which is implementing
this algorithm currently in Python on top of PyDiGraph (it will also be
used by a future method collect_1q_runs which is being added in
Qiskit/qiskit#5431).
mtreinish added a commit to Qiskit/rustworkx that referenced this pull request Dec 3, 2020
@mtreinish @lcapelluto @itoko
Add collect_runs() function (#206)
6782e55 
* Add collect_runs() function

This commit adds a new algorithms function, collect_runs(), which is
used to find a list of runs in a PyDiGraph object that match a specified
condition. A run in this context is any linear path inside the graph.
The condition is specified using a python function that will be passed a
node's data payload/weight object and will return whether it matches the
condition or not. The intended use case for this function is with
qiskit-terra's PyDiGraph method collect_runs() which is implementing
this algorithm currently in Python on top of PyDiGraph (it will also be
used by a future method collect_1q_runs which is being added in
Qiskit/qiskit#5431).

* Handle multiple edges to a single successor node

There was a bug in the collect_runs() method when there were multiple
edges between a graph and its single successor node. Since
neighbors_directed() returns an iterator over the edges and not the
nodes it would have multiple entries in the succesors list despite only
being a single node. This commit addresses that edge case by building a
HashSet of the node indices in the succesors list and checking the
length of that instead of using the raw successors list. That will
contain no duplicate nodes and will identify if there is a path instead
of splitting on multiple edges incorrectly.

* Add initial tests

* Add more tests

* Use Vec.dedup() and graph access instead of Vec of tuples and HashSet

* Update src/lib.rs

* Apply suggestions from code review

Co-authored-by: Lauren Capelluto <laurencapelluto@gmail.com>

* Document and test nodes can only exist in a single run

This commit documents and adds tests to assert that a node can only be
in a single run in the output from the collect_runs() method. In terra
the equivalent python function didn't have this additional constraint
so this commit attempts to make it clear so the differences between
the two implementations are understood by users.

Co-authored-by: Lauren Capelluto <laurencapelluto@gmail.com>
Co-authored-by: Toshinari Itoko <itoko@jp.ibm.com>
@kdk kdk mentioned this pull request Dec 3, 2020
Open
@mtreinish
Improve 1q decomposition pass with multiple matching basis
9bf88b5 
This commit makes an improvement to the Optimize1qGatesDecomposition
pass to improve the output from it in two scenarios. The first scenario
is if there is an over complete basis where there are multiple choices
for decomposition basis. Previously in such scenarios only the first
matching subset would be used. This improves that by finding runs in all
matching decomposition basis to simplify any runs in those gate sets.
The second improvement made here is that the decomposition is only used
if it results in a decrease in depth. There were scenarios where the
general decomposition of a run would result in more gates than the
input to the OneQubitEulerDecomposer (typically 3 gates vs 2 input). In
those cases we shouldn't use the decomposition and instead rely on the
original run because we're adding gates which is the opposit of the
expected behavior.
@mtreinish
Add DAGCircuit method to get 1q op node runs
55f34f3 
This commit adds a new DAGCircuit method, collect_1q_runs, which behaves
like collect_runs(), but instead of collecting runs with gates on a name
filter it looks at the number of qubits and returns runs of any op node
that operate on a single qubit. This will be used by the optimize 1q
decomposition pass to collect arbitrary single qubit gate runs and
decompose them over the basis sets.
@mtreinish
Switch to use all 1q runs instead of those just matching basis
346baaa
@mtreinish
Use retworkx.collect_runs for collect_1q_runs
9fcb607
@mtreinish
Move parameterized gate split to collect_1q_runs
dbd2fef
@mtreinish
Reverse loops so basis is the inner loop and runs the outer
01e76c2
@mtreinish mtreinish force-pushed the use-multiple-basis-optimize-1q-decompose branch from 8b0793f to 01e76c2 Compare December 9, 2020 20:09
@kdk kdk added the automerge label Dec 10, 2020
@mergify mergify bot merged commit 5a1768f into Qiskit:master Dec 11, 2020
@ajavadia ajavadia mentioned this pull request Dec 17, 2020
Open
@mtreinish mtreinish deleted the use-multiple-basis-optimize-1q-decompose branch January 4, 2021 15:32
mtreinish added a commit to mtreinish/qiskit-core that referenced this pull request Jan 19, 2021
@mtreinish
Improve 1q decomposition pass with multiple matching basis (Qiskit#5431)
8d09385 
* Improve 1q decomposition pass with multiple matching basis

This commit makes an improvement to the Optimize1qGatesDecomposition
pass to improve the output from it in two scenarios. The first scenario
is if there is an over complete basis where there are multiple choices
for decomposition basis. Previously in such scenarios only the first
matching subset would be used. This improves that by finding runs in all
matching decomposition basis to simplify any runs in those gate sets.
The second improvement made here is that the decomposition is only used
if it results in a decrease in depth. There were scenarios where the
general decomposition of a run would result in more gates than the
input to the OneQubitEulerDecomposer (typically 3 gates vs 2 input). In
those cases we shouldn't use the decomposition and instead rely on the
original run because we're adding gates which is the opposit of the
expected behavior.

* Add DAGCircuit method to get 1q op node runs

This commit adds a new DAGCircuit method, collect_1q_runs, which behaves
like collect_runs(), but instead of collecting runs with gates on a name
filter it looks at the number of qubits and returns runs of any op node
that operate on a single qubit. This will be used by the optimize 1q
decomposition pass to collect arbitrary single qubit gate runs and
decompose them over the basis sets.

* Switch to use all 1q runs instead of those just matching basis

* Use retworkx.collect_runs for collect_1q_runs

* Move parameterized gate split to collect_1q_runs

* Reverse loops so basis is the inner loop and runs the outer

* Ensure collect_1q_runs doesn't collect measurements

* Only create temp circuit once per run

* Move lone identity optimization into outer loop

* Update qiskit/transpiler/passes/optimization/optimize_1q_decomposition.py

* Fix lint

* Use qc._append instead of qc.append

Conflicts/Changes:

For backport this makes 1 key change, the collect_1q_runs method is
rewritten to be pure python instead of leveraging the retworkx
collect_runs() method which was added in a newer version of retworkx
than the minimum allowed version. This also partially pulls in the
changes made in Qiskit#5570, but only some were backportable since the others
relied on attributes that don't exist in gate objects in 0.16.x.

(cherry picked from commit 5a1768f)
@mtreinish mtreinish mentioned this pull request Jan 19, 2021
Merged
mergify bot pushed a commit that referenced this pull request Jan 21, 2021
@mtreinish
[Stable] Improve 1q decomposition pass with multiple matching basis (#…
dc7ac53 
…5431) (#5655)

* Improve 1q decomposition pass with multiple matching basis (#5431)

* Improve 1q decomposition pass with multiple matching basis

This commit makes an improvement to the Optimize1qGatesDecomposition
pass to improve the output from it in two scenarios. The first scenario
is if there is an over complete basis where there are multiple choices
for decomposition basis. Previously in such scenarios only the first
matching subset would be used. This improves that by finding runs in all
matching decomposition basis to simplify any runs in those gate sets.
The second improvement made here is that the decomposition is only used
if it results in a decrease in depth. There were scenarios where the
general decomposition of a run would result in more gates than the
input to the OneQubitEulerDecomposer (typically 3 gates vs 2 input). In
those cases we shouldn't use the decomposition and instead rely on the
original run because we're adding gates which is the opposit of the
expected behavior.

* Add DAGCircuit method to get 1q op node runs

This commit adds a new DAGCircuit method, collect_1q_runs, which behaves
like collect_runs(), but instead of collecting runs with gates on a name
filter it looks at the number of qubits and returns runs of any op node
that operate on a single qubit. This will be used by the optimize 1q
decomposition pass to collect arbitrary single qubit gate runs and
decompose them over the basis sets.

* Switch to use all 1q runs instead of those just matching basis

* Use retworkx.collect_runs for collect_1q_runs

* Move parameterized gate split to collect_1q_runs

* Reverse loops so basis is the inner loop and runs the outer

* Ensure collect_1q_runs doesn't collect measurements

* Only create temp circuit once per run

* Move lone identity optimization into outer loop

* Update qiskit/transpiler/passes/optimization/optimize_1q_decomposition.py

* Fix lint

* Use qc._append instead of qc.append

Conflicts/Changes:

For backport this makes 1 key change, the collect_1q_runs method is
rewritten to be pure python instead of leveraging the retworkx
collect_runs() method which was added in a newer version of retworkx
than the minimum allowed version. This also partially pulls in the
changes made in #5570, but only some were backportable since the others
relied on attributes that don't exist in gate objects in 0.16.x.

(cherry picked from commit 5a1768f)

* Fix lint

* Simplify collect_1q_runs

* Move pylint disable to just collect_1q_runs

* Reorder 1q node checks for performance
@kdk kdk added the Changelog: None Do not include in changelog label Mar 31, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kdk kdk kdk approved these changes

Assignees
No one assigned
Labels
Changelog: None Do not include in changelog
Projects
None yet
Milestone
0.17
Development

Successfully merging this pull request may close these issues.
2 participants
@mtreinish @kdk