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

Avoid collecting non atomic nodes in 1q runs #5570

Merged
merged 5 commits into from
Jan 7, 2021

Conversation

paolob67
Copy link
Contributor

@paolob67 paolob67 commented Jan 1, 2021

Summary

Fixes #5556
Avoid collecting operations such as reset and delay by adding a test on decompositions in the filter conditions.

Details and comments

What we want to do here is to define atomic ops as those that do not have decompositions in the definition. This excludes those two operations but take note that it also excludes the identity gate. The patch consists in adding an additional condition to the filter.

@paolob67 paolob67 requested a review from a team as a code owner January 1, 2021 10:05
@taalexander
Copy link
Contributor

This solution seems to work for #5543 while potentially leaving the door open in the future for decomposing "atomic" operations.

@kdk kdk changed the title Fixing issue 5543 avoid collecting non atomic nodes in 1q runs Avoid collecting non atomic nodes in 1q runs Jan 6, 2021
@kdk kdk added this to the 0.17 milestone Jan 6, 2021
Copy link
Member

@kdk kdk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @paolob67 , one comment based on discussion from #5543 . Otherwise, looks good!

Co-authored-by: Kevin Krsulich <kevin@krsulich.net>
@kdk kdk added the automerge label Jan 7, 2021
@mtreinish mtreinish added Changelog: Bugfix Include in the "Fixed" section of the changelog stable backport potential The bug might be minimal and/or import enough to be port to stable and removed stable backport potential The bug might be minimal and/or import enough to be port to stable labels Jan 7, 2021
@mergify mergify bot merged commit e193de0 into Qiskit:master Jan 7, 2021
@paolob67 paolob67 deleted the issue#5543 branch January 8, 2021 14:30
mtreinish added a commit to mtreinish/qiskit-core that referenced this pull request Jan 19, 2021
* 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)
mergify bot pushed a commit that referenced this pull request Jan 21, 2021
…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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Changelog: Bugfix Include in the "Fixed" section of the changelog
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Transpilation fails with snapshot instruction
4 participants