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

The number of CZ gates after transpilation (optimization level=2 and 3) increases drastically for 1.3.0rc1 and main branch compared with 1.2.4 #13459

Closed
t-imamichi opened this issue Nov 20, 2024 · 3 comments · Fixed by #13463
Assignees
Labels
bug Something isn't working
Milestone

Comments

@t-imamichi
Copy link
Member

Environment

  • Qiskit version: 1.3.0rc1, main
  • Python version: 3.12.7
  • Operating system: macOS 15.1.1

What is happening?

The number of CZ gates after transpilation with optimization_level 2 and 3 increases drastically for 1.3.0rc1 and main branch compared with 1.2.4

How can we reproduce the issue?

I attach a zip file with qaoa.qpy that includes the target circuit generated from QAOA.
I tried to make a minimal example, but I could not do it unfortunately.

Need to unzip the attachment file qaoa.zip to extract qaoa.qpy.

from qiskit import generate_preset_pass_manager, qpy, __version__
from qiskit_ibm_runtime.fake_provider import FakeTorino

with open("qaoa.qpy", "rb") as file:
    circuit = qpy.load(file)[0]
print(__version__)
print("before")
print(circuit.count_ops(), "\n")

backend = FakeTorino()
layout = [0, 1, 2, 3, 4, 5, 6, 7, 8, 17, 27, 26, 25, 24, 23, 22, 21]

for level in [1, 2, 3]:
    pm = generate_preset_pass_manager(
        optimization_level=level,
        backend=backend,
        initial_layout=layout,
        layout_method="trivial",
        routing_method="none",
    )
    tcircuit = pm.run(circuit)
    print("optimization_level", level)
    print(tcircuit.count_ops(), "\n")
1.2.4
before
OrderedDict({'PauliEvolution': 91, 'swap': 90, 'h': 17, 'rz': 17, 'rx': 17, 'measure': 17})

optimization_level 1
OrderedDict({'rz': 901, 'sx': 860, 'cz': 452, 'measure': 17})

optimization_level 2
OrderedDict({'rz': 701, 'sx': 697, 'cz': 308, 'measure': 17, 'x': 3})

optimization_level 3
OrderedDict({'rz': 701, 'sx': 697, 'cz': 308, 'measure': 17, 'x': 3})
1.3.0rc1
before
OrderedDict({'PauliEvolution': 91, 'swap': 90, 'h': 17, 'rz': 17, 'rx': 17, 'measure': 17})

optimization_level 1
OrderedDict({'rz': 901, 'sx': 860, 'cz': 452, 'measure': 17})

optimization_level 2
OrderedDict({'sx': 848, 'rz': 595, 'cz': 444, 'measure': 17})

optimization_level 3
OrderedDict({'sx': 848, 'rz': 595, 'cz': 444, 'measure': 17})
2.0.0.dev0+94cea41
before
OrderedDict({'PauliEvolution': 91, 'swap': 90, 'h': 17, 'rz': 17, 'rx': 17, 'measure': 17})

optimization_level 1
OrderedDict({'rz': 901, 'sx': 860, 'cz': 452, 'measure': 17})

optimization_level 2
OrderedDict({'sx': 848, 'rz': 595, 'cz': 444, 'measure': 17})

optimization_level 3
OrderedDict({'sx': 848, 'rz': 595, 'cz': 444, 'measure': 17})

What should happen?

The number of CZ gates should be as same as that of 1.2.4.

Any suggestions?

I tried callback to check the count ops for each pass and found that something is happening around ConsolidateBlocks.
Collect2qBlocks seems to disappear for the main branch.

from qiskit import generate_preset_pass_manager, qpy, __version__
from qiskit_ibm_runtime.fake_provider import FakeTorino

with open("qaoa.qpy", "rb") as file:
    circuit = qpy.load(file)[0]
print(__version__)

backend = FakeTorino()
layout = [0, 1, 2, 3, 4, 5, 6, 7, 8, 17, 27, 26, 25, 24, 23, 22, 21]

def callback(**kwargs):
    print(kwargs["pass_"])
    print(kwargs["dag"].count_ops())

pm = generate_preset_pass_manager(
    optimization_level=2,
    backend=backend,
    initial_layout=layout,
    layout_method="trivial",
    routing_method="none",
)
tcircuit = pm.run(circuit, callback=callback)
1.2.4
...
<qiskit.transpiler.passes.basis.basis_translator.BasisTranslator object at 0x114984b90>
{'rz': 921, 'sx': 955, 'cz': 452, 'measure': 17}
<qiskit.transpiler.passes.optimization.collect_2q_blocks.Collect2qBlocks object at 0x11494ce90>
{'rz': 921, 'sx': 955, 'cz': 452, 'measure': 17}
<qiskit.transpiler.passes.optimization.consolidate_blocks.ConsolidateBlocks object at 0x114a28bc0>
{'rz': 517, 'sx': 302, 'cz': 164, 'measure': 17, 'unitary': 72}
<qiskit.transpiler.passes.synthesis.unitary_synthesis.UnitarySynthesis object at 0x1142a54f0>
{'rz': 1141, 'sx': 720, 'cz': 308, 'x': 3, 'measure': 17}
...
2.0.0.dev0+94cea41
...
<qiskit.transpiler.passes.basis.basis_translator.BasisTranslator object at 0x13ba103b0>
{'rz': 921, 'sx': 955, 'cz': 452, 'measure': 17}
<qiskit.transpiler.passes.optimization.consolidate_blocks.ConsolidateBlocks object at 0x139e60ad0>
{'rz': 889, 'sx': 915, 'cz': 436, 'measure': 17, 'unitary': 4}
<qiskit.transpiler.passes.synthesis.unitary_synthesis.UnitarySynthesis object at 0x138c8f5f0>
{'rz': 933, 'sx': 939, 'cz': 444, 'measure': 17}
...

full log is available at https://gist.github.com/t-imamichi/eb35f536117e45dd89293e7e979132cb

@t-imamichi t-imamichi added the bug Something isn't working label Nov 20, 2024
@mtreinish
Copy link
Member

Thanks, this is pointing to another in the oxidized consolidate blocks. The lack of collect2qblocks is expected because that's now run inline consolidate blocks all in rust for efficiency. But the fact that it's only collecting 4 unitaries when in 1.2.4 it returned 72 is an issue.

@mtreinish mtreinish added this to the 1.3.0 milestone Nov 20, 2024
@mtreinish
Copy link
Member

The issue here is a difference between how Rust's two qubit decomposer interface works and the python api that wraps it. In rust we call decomposer.gate_name(): https://github.com/Qiskit/qiskit/blob/main/crates/accelerate/src/consolidate_blocks.rs#L128 to check the number of basis gates in the block. However, the return of that method will be USER_GATE for anything outside of a cx gate (which has special handling for a pulse optimal decomposition) so that the python space circuit generation can handle a custom gate. This differs from what was done in Python because the analogous call https://github.com/Qiskit/qiskit/blob/1.2.4/qiskit/transpiler/passes/optimization/consolidate_blocks.py#L97 which will return the actual name cz. This bug means that consolidate blocks never triggers for cz blocks because it's not counting the right gates.

@mtreinish mtreinish self-assigned this Nov 20, 2024
mtreinish added a commit to mtreinish/qiskit-core that referenced this issue Nov 20, 2024
The ConsolidateBlocks pass was ported to rust in Qiskit#13368 and as part of
that implementation a small behavior difference between the rust and
python interfaces was causing the pass to not work correctly with non-CX
gates. The internal 2q decomposer interface stores a sentinel string for
the kak gate which is used to tell the python space constructor use the
python defined gate object. However in the pass code we weren't
factoring this difference in, and for non-CX gates we were evaluating
the basis count as the number of gates with that sentinel value name
(which is almost always zero) and this was preventing the pass from
consolidating many blocks that should have been. This commit fixes this
issue by taking the name from python space and passing it through to the
rust portion of the code and using that for the comparison.

Fixes Qiskit#13459
github-merge-queue bot pushed a commit that referenced this issue Nov 20, 2024
The ConsolidateBlocks pass was ported to rust in #13368 and as part of
that implementation a small behavior difference between the rust and
python interfaces was causing the pass to not work correctly with non-CX
gates. The internal 2q decomposer interface stores a sentinel string for
the kak gate which is used to tell the python space constructor use the
python defined gate object. However in the pass code we weren't
factoring this difference in, and for non-CX gates we were evaluating
the basis count as the number of gates with that sentinel value name
(which is almost always zero) and this was preventing the pass from
consolidating many blocks that should have been. This commit fixes this
issue by taking the name from python space and passing it through to the
rust portion of the code and using that for the comparison.

Fixes #13459
mergify bot pushed a commit that referenced this issue Nov 20, 2024
The ConsolidateBlocks pass was ported to rust in #13368 and as part of
that implementation a small behavior difference between the rust and
python interfaces was causing the pass to not work correctly with non-CX
gates. The internal 2q decomposer interface stores a sentinel string for
the kak gate which is used to tell the python space constructor use the
python defined gate object. However in the pass code we weren't
factoring this difference in, and for non-CX gates we were evaluating
the basis count as the number of gates with that sentinel value name
(which is almost always zero) and this was preventing the pass from
consolidating many blocks that should have been. This commit fixes this
issue by taking the name from python space and passing it through to the
rust portion of the code and using that for the comparison.

Fixes #13459

(cherry picked from commit c792426)
github-merge-queue bot pushed a commit that referenced this issue Nov 20, 2024
The ConsolidateBlocks pass was ported to rust in #13368 and as part of
that implementation a small behavior difference between the rust and
python interfaces was causing the pass to not work correctly with non-CX
gates. The internal 2q decomposer interface stores a sentinel string for
the kak gate which is used to tell the python space constructor use the
python defined gate object. However in the pass code we weren't
factoring this difference in, and for non-CX gates we were evaluating
the basis count as the number of gates with that sentinel value name
(which is almost always zero) and this was preventing the pass from
consolidating many blocks that should have been. This commit fixes this
issue by taking the name from python space and passing it through to the
rust portion of the code and using that for the comparison.

Fixes #13459

(cherry picked from commit c792426)

Co-authored-by: Matthew Treinish <[email protected]>
@t-imamichi
Copy link
Member Author

Thank you for fixing the issue. I confirmed that it is resolved by 1.3.0rc2.

1.3.0rc2
before
OrderedDict({'PauliEvolution': 91, 'swap': 90, 'h': 17, 'rz': 17, 'rx': 17, 'measure': 17})

optimization_level 1
OrderedDict({'rz': 901, 'sx': 860, 'cz': 452, 'measure': 17})

optimization_level 2
OrderedDict({'sx': 696, 'rz': 636, 'cz': 308, 'measure': 17, 'x': 3})

optimization_level 3
OrderedDict({'sx': 696, 'rz': 636, 'cz': 308, 'measure': 17, 'x': 3})

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