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

Optimize1qGatesDecomposition doesn't account for ideal gates correctly #10568

Closed
mtreinish opened this issue Aug 4, 2023 · 3 comments · Fixed by #11351
Closed

Optimize1qGatesDecomposition doesn't account for ideal gates correctly #10568

mtreinish opened this issue Aug 4, 2023 · 3 comments · Fixed by #11351
Assignees
Labels
bug Something isn't working

Comments

@mtreinish
Copy link
Member

mtreinish commented Aug 4, 2023

Environment

  • Qiskit Terra version: 0.25.0 (but also earlier releases and main)
  • Python version: 3.11
  • Operating system: Linux

What is happening?

If a target has ideal gates with all error rates of 0 reported the output from the pass will translate from one gate to another unecessarily.

How can we reproduce the issue?

from qiskit.transpiler import Target, InstructionProperties
from qiskit.circuit import QuantumCircuit, Parameter
from qiskit.circuit.library import UGate, HGate
from qiskit.compiler import transpile

target = Target(num_qubits=1)
target.add_instruction(HGate(), {(0,): InstructionProperties(error=0)})
target.add_instruction(
    UGate(Parameter("a"), Parameter("b"), Parameter("c")), {(0,): InstructionProperties(error=0)}
)

qc = QuantumCircuit(1)
qc.h(0)

print(transpile(qc, target=target))

What should happen?

The output should remain as h because it's in the target and has the same error rate and gate count as UGate. While both outputs are valid and the transpiler is doing it's job, in this situation it's a bit unexpected for the pass to translate the gate when the input was still valid and by the error heuristics no better or worse than the translated output.

Any suggestions?

I think we probably need to add a condition to fallback to the input in the substitution checks if the output of all the synthesis routines are of equal weight.

@burgholzer
Copy link
Contributor

Following up on the discussion in #10592.
I'd like to work on this. I can't yet guarantee that I'll have the necessary bandwidth in the coming days, but I'll see what I can do!

mtreinish added a commit to mtreinish/qiskit-core that referenced this issue Nov 30, 2023
This commit fixes an issue in Optimize1qGatesDecomposition where the
pass would defer to synthesized gates if the output from synthesis was
ideal even if the input gate was also ideal. This typically comes up in
simulators where there are no error rates for any gates and everything
is listed as ideal. This would cause the transpiler to translate gates
unnecessarily which was unexpected. This commit fixes this by adding an
additional check to the subsitution condition to ensure we're only
substituting a gate if it's not in the target (when they're all ideal).

Fixes Qiskit#10568
@mtreinish
Copy link
Member Author

I hope you don't mind I was hitting this more frequently now that qiskit-aer is using BackendV2 so I pushed up: #11351 to fix this issue.

github-merge-queue bot pushed a commit that referenced this issue Dec 1, 2023
…ion (#11351)

* Don't substitute ideal gates in target with Optimize1qGatesDecomposition

This commit fixes an issue in Optimize1qGatesDecomposition where the
pass would defer to synthesized gates if the output from synthesis was
ideal even if the input gate was also ideal. This typically comes up in
simulators where there are no error rates for any gates and everything
is listed as ideal. This would cause the transpiler to translate gates
unnecessarily which was unexpected. This commit fixes this by adding an
additional check to the subsitution condition to ensure we're only
substituting a gate if it's not in the target (when they're all ideal).

Fixes #10568

* Update releasenotes/notes/fix-optimize-1q-sim-407b88e45e6062b6.yaml

Co-authored-by: Jake Lishman <jake@binhbar.com>

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

* Fix formatting

---------

Co-authored-by: Jake Lishman <jake@binhbar.com>
mergify bot pushed a commit that referenced this issue Dec 1, 2023
…ion (#11351)

* Don't substitute ideal gates in target with Optimize1qGatesDecomposition

This commit fixes an issue in Optimize1qGatesDecomposition where the
pass would defer to synthesized gates if the output from synthesis was
ideal even if the input gate was also ideal. This typically comes up in
simulators where there are no error rates for any gates and everything
is listed as ideal. This would cause the transpiler to translate gates
unnecessarily which was unexpected. This commit fixes this by adding an
additional check to the subsitution condition to ensure we're only
substituting a gate if it's not in the target (when they're all ideal).

Fixes #10568

* Update releasenotes/notes/fix-optimize-1q-sim-407b88e45e6062b6.yaml

Co-authored-by: Jake Lishman <jake@binhbar.com>

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

* Fix formatting

---------

Co-authored-by: Jake Lishman <jake@binhbar.com>
(cherry picked from commit f12db3b)
@burgholzer
Copy link
Contributor

I hope you don't mind I was hitting this more frequently now that qiskit-aer is using BackendV2 so I pushed up: #11351 to fix this issue.

Not at all. Many thanks for the fix and sorry that I didn't manage to get to it sooner 🥲

github-merge-queue bot pushed a commit that referenced this issue Dec 1, 2023
…ion (#11351) (#11359)

* Don't substitute ideal gates in target with Optimize1qGatesDecomposition

This commit fixes an issue in Optimize1qGatesDecomposition where the
pass would defer to synthesized gates if the output from synthesis was
ideal even if the input gate was also ideal. This typically comes up in
simulators where there are no error rates for any gates and everything
is listed as ideal. This would cause the transpiler to translate gates
unnecessarily which was unexpected. This commit fixes this by adding an
additional check to the subsitution condition to ensure we're only
substituting a gate if it's not in the target (when they're all ideal).

Fixes #10568

* Update releasenotes/notes/fix-optimize-1q-sim-407b88e45e6062b6.yaml

Co-authored-by: Jake Lishman <jake@binhbar.com>

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

* Fix formatting

---------

Co-authored-by: Jake Lishman <jake@binhbar.com>
(cherry picked from commit f12db3b)

Co-authored-by: Matthew Treinish <mtreinish@kortar.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants