Fixed IQAE bug mentioned in issue 9280

[shifubear]

Summary

Reopening a PR due to account conflicts present in PR #9289.

Details and comments

Fixed the bug mentioned in issue #9280, and added more tests to increase robustness for phase estimation algorithms.

[qiskit-bot]

Thank you for opening a new pull request.

Before your PR can be merged it will first need to pass continuous integration tests and be reviewed. Sometimes the review process can be slow, so please be patient.

While you're waiting, please feel free to review other open PRs. While only a subset of people are authorized to approve pull requests for merging, everyone is encouraged to review open pull requests. Doing reviews helps reduce the burden on the core team and helps make the project's code better for everyone.

One or more of the the following people are requested to review this:

• @Qiskit/terra-core
• @manoelmarques
• @woodsp-ibm

[shifubear]

Hi @woodsp-ibm, I reopened a new PR with the same changes here since there were some issues in the history of PR #9289 that seemed challenging to resolve.

[Cryoris]

LGTM but @jlapeyre could you also have a look? 🙂

[jlapeyre]

@shifubear Thanks for sticking with this through the administrative difficulty.

It looks fine to me. You may want to consider doing this instead

        qubits = [unitary.num_qubits]
        qubits.extend(range(0, unitary.num_qubits))
        qc = qc.compose(unitary_power, qubits)

This is a significantly faster way to construct the list. In general, I advocate using an idiomatic best way to write something. In part because the code serves as a model for further development. Which way to write it is best?

• extend is faster in this case
• The increase in performance won't matter in this function, because surrounding code is much slower. But it's good to habitually use the best choice. But it's not clear this is the best choice because...
• + is more readable and easily understandable in this case.

Unfortunately, extend returns None rather than the list. If it did return the list, then you could write

qc = qc.compose(unitary_power, [unitary.num_qubits].extend(range(0, unitary.num_qubits)))

and you'd have something that is both faster and no less readable. But alas this is not the case.

On balance, I favor the code as you have written in, with +, because the readability outweighs the performance. Do you have an opinion @Cryoris ?

[shifubear]

@jlapeyre Of course! Wanted to contribute so I'm glad there was a workaround.

I prefer optimizing for readability in this instance, especially since the surrounding code is much slower as you mentioned. Also, my intuition is that the length of the qubits list will not be long enough for there to be a significant performance difference in the near future.
With that said though, I was unaware of the extend method so I will keep note of that for future PRs!

[jlapeyre]

I don't think there is a conceivable scenario where the performance of this line would matter in this method.

Once the questions on the release note are addressed, I think this is good to go.

[shifubear]

Hi @jlapeyre, I added the suggested changes on the release notes!

[jlapeyre]

There should be a newline at the end of this file. The red circle warns about this.

[shifubear]

I added a newline but it looks like some tests are still failing. I can't seem to find what exactly is the cause. Do you have any pointers on how to find the error?

[ElePT]

Hi @shifubear, you should add angle brackets to your reno hyperlink definition, sphinx expects something like:

`reference_to_link_in_text <https://link.com>`__

[coveralls]

Pull Request Test Coverage Report for Build 4611005501

• 1 of 1 (100.0%) changed or added relevant line in 1 file are covered.
• 118 unchanged lines in 9 files lost coverage.
• Overall coverage decreased (-0.05%) to 85.351%

---

Files with Coverage Reduction	New Missed Lines	%
qiskit/primitives/backend_estimator.py	1	95.37%
qiskit/primitives/backend_sampler.py	1	97.73%
qiskit/pulse/utils.py	2	94.29%
qiskit/transpiler/preset_passmanagers/level3.py	5	92.5%
qiskit/circuit/library/standard_gates/multi_control_rotation_gates.py	10	93.38%
qiskit/circuit/quantumcircuitdata.py	12	88.18%
qiskit/transpiler/passmanager.py	12	93.68%
qiskit/test/base.py	18	84.03%
qiskit/circuit/quantumcircuit.py	57	94.17%

Totals
Change from base Build 4569011817:	-0.05%
Covered Lines:	67114
Relevant Lines:	78633

---

💛 - Coveralls

[Cryoris]

LGTM thanks for the fix!