Add transpile_operator function to primitives.utils

[ikkoham]

Summary

Details and comments

[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
• @ajavadia
• @ikkoham
• @levbishop
• @t-imamichi

[coveralls]

Pull Request Test Coverage Report for Build 4743816955

• 12 of 12 (100.0%) changed or added relevant lines in 1 file are covered.
• 39 unchanged lines in 6 files lost coverage.
• Overall coverage decreased (-0.04%) to 85.892%

---

Files with Coverage Reduction	New Missed Lines	%
crates/qasm2/src/expr.rs	1	93.76%
qiskit/circuit/tools/pi_check.py	1	91.23%
qiskit/transpiler/passes/synthesis/unitary_synthesis.py	1	95.14%
crates/qasm2/src/lex.rs	3	91.14%
qiskit/pulse/library/waveform.py	3	91.67%
crates/qasm2/src/parse.rs	30	95.71%

Totals
Change from base Build 4737986268:	-0.04%
Covered Lines:	71058
Relevant Lines:	82729

---

💛 - Coveralls

[woodsp-ibm]

I have seen users try to do this when using the applications and passing in what they consider an already optimized ansatz they have transpiled themselves and failing as the resultant circuit, with swaps, is wider (more qubits) than the original operator. I had wondered about whether an Estimator could take some mapping the user passed in that mapped the original qubits to the ones in the circuit and did whatever it needed to do internally. From an application standpoint the stacks today just pass down operators into algorithms pre-built out by the user configured with primitives. Currently these have no visibility to the notion of whether a circuit might have been transpiled and the user has setup the Estimator to skip transpilation and passed some pre-transpiled circuit as an ansatz. If a user uses an Estimator directly then what is proposed here works; given how the application stacks are today and the user preparing an algorithm with a primitive something needs to be done to cater to that too. Arguably the notion of transpilation or not could be propagated higher up the stack and have each algo deal with it that uses an Estimator. From my perspective it would be nicer if it whatever was need to be done was more self-contained with the primitive, such that it would deal with the matching of the operator to the transpiled circuit based on the user configuring the primitive according - ie skip_translation and whatever was needed to understand the circuit mapping/layout.

[ikkoham]

TBH, we did not expect that transpiled circuits would be passed on, and the introduction of skip_transpilation is merely a work-around for a specific problem. However, as such use cases have arisen, final layout in QuantumCircuit has come to be exposed.
I think it is something that is repeatedly modified ad hoc.

I would like to make a fundamental fix, but the main bottleneck is that the layout of a transpiled circuit does not remember the original qubits. Therefore, I am passing original_qubits in this function. If we can solve this problem, I can modify the internal program of BackendEstimator to work with transpiled circuits and passing raw observables, but it is difficult at the moment.
Sorry that this utility is also ad hoc. But there are so many users. I will leave it up to the reviewer to decide whether to merge this PR or not. It is not a long function, so I think it could be copied and pasted for use.

[mtreinish]

but the main bottleneck is that the layout of a transpiled circuit does not remember the original qubits.

The original qubits should be in the TranspileLayout object as part of the input_qubit_mapping field. That contains a dict mapping the virtual qubits to the positional index in the input circuit to transpile(). I hit the same thing in Operator.from_circuit when trying to get that to reverse the layout permutation and that was why we created the TranspileLayout class. You can see it used here: https://github.com/Qiskit/qiskit-terra/blob/main/qiskit/quantum_info/operators/operator.py#L254-L258

[Cryoris]

Overall LGTM, I just have some minor comments below 🙂

[Cryoris]

I'm not sure this will be found as it is, I think it would be safer to either add the full path, or use .transpile_operator only 🙂

Suggested change

	Add :func:`~primitives.utils.transpile_operator` function.
	Add :func:`~qiskit.primitives.utils.transpile_operator` function.

[Cryoris]

Could you add an additional test that checks that trans_op equals the expected operator? Testing that the values are correct is also good, but there are other steps included which could affect the test, so I think it would be good to have a very specific one too 🙂

[ikkoham]

sure.

[Cryoris]

Should we use collections.abc instead of typing here?

[ikkoham]

Oh... What’s wrong with me

[ikkoham]

@mtreinish Thank you. I checked TranspileLayout.input_qubit_mapping, but how do you distinguish whether they are original qubits or additional (ancilla) qubits?

[Cryoris]

I'm wondering if transpile_operator is an intuitive name or if this should be called something like pad_operator or something along these lines?

[mtreinish]

I just opened a PR #10835 which I think will make this a lot easier to adjust based on the layout from a transpiled circuit inside the estimator.

[nonhermitian]

I think it is best if this is not inside the primitives module as it is independent of the use of primitives. Also why is it not just the operator and layout that is required? Perhaps this is now the case with the changes made in #10835 ?

[ikkoham]

#10947 is better than this PR.