Update gate.inverse() to return AnnotatedOperation

[alexanderivrii]

Summary

This PR adds a new argument annotated to all the "inverse" methods. This is a follow-up to #11433 (and should be rebased onto that).

Details and comments

Note that even though we have to update the interface of many standard gates that implement inverse, in practice inverses of standard gates are also standard gates, and we should always prefer these over an "inverse-annotated" operation. For instance, the inverse of the CX-gate is again the CX-gate, and the inverse of the S-gate is the Sdg-gate (and in addition all of these are singleton gates).

I believe that for the immediate release it makes sense to either include both this and #11433 together or to include neither. I have updated the priority to that of #11433.

[qiskit-bot]

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

• @Cryoris
• @Qiskit/terra-core
• @ajavadia

[coveralls]

Pull Request Test Coverage Report for Build 7744292754

• -6 of 103 (94.17%) changed or added relevant lines in 46 files are covered.
• 11 unchanged lines in 2 files lost coverage.
• Overall coverage increased (+0.002%) to 89.344%

---

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
qiskit/circuit/controlledgate.py	4	5	80.0%
qiskit/circuit/library/generalized_gates/mcg_up_to_diagonal.py	7	8	87.5%
qiskit/circuit/library/generalized_gates/uc.py	8	9	88.89%
qiskit/circuit/annotated_operation.py	1	4	25.0%

Files with Coverage Reduction	New Missed Lines	%
crates/qasm2/src/lex.rs	5	91.69%
crates/qasm2/src/parse.rs	6	97.62%

Totals
Change from base Build 7744260861:	0.002%
Covered Lines:	60168
Relevant Lines:	67344

---

💛 - Coveralls

[mtreinish]

Overall this looks fine, it's a similar addition to #11433. I just have a few inline comments. The other higher level question is around the standard gates, most of them add the new kwarg to be compliant with the interface inherited from Instruction, but they then ignore the value of the annotated kwarg and always return the discrete definition of the inverse. You called this out in the PR summary, but I'm wondering if we still want to return return super().inverse(annotated=annotated) if it's true even if there isn't much of a use case. I'm worried it might be confusing for users if they call CXGate().inverse(annotated=True and just get a CXGate back. Even though cx is a self inverse maybe the use case for annotated in that case is that they want to pair with other modifiers for some potential optimization in the transpiler. TBH, I realize that argument is a bit of a stretch I'm mostly just worried about api consistency with the new option

Also this is missing a release note documenting this new feature.

[alexanderivrii]

Thanks! I have added the release notes and a few tests (somehow I forgot to do this either).

At the moment I can't think of a scenario where keeping an inverse SGate as an AnnotatedOperation instead of an SdgGate would be useful (though I get your point on potentially combining modifiers).

I was actually thinking earlier that we may want to have 3 different values for annotated:

1. Do not create AnnotatedOperation unless the operation does not make sense otherwise (e.g., control/inverse of an annotated operation)
2. Always create AnnotatedOperation
3. Choose whatever leads to simpler gates/circuits

We need 1 to preserve the existing behavior. We want 3 in practice. But I could not convince myself that we really need 2. So, not to over-complicate things, what's implemented right now has annotated=False corresponding to 1 and annotated=True corresponding to 3. I have tried to clarify this both in the release notes and in the documentation of Instruction.inverse(). What do you think?

[mtreinish]

I'm ok with that decision as long as we document it clearly. I started leaving inline suggestions to update the docs but then decided we can do that in a follow up PR after RC1. I think the only blocker on this right now is the CI failure which I think is coming from the type hint and sphinx not being able to resolve Instruction correctly.

[mtreinish]

It's unfortunate that there are 2 Instruction classes in qiskit one for circuits and the other in pulse. The ci docs failure is because sphinx couldn't tell them apart with just the Instruction class listed here. Sphinx doesn't really understand type hinting in the same manner that python does apparently, because in python there is enough context to know that Instruction here is the class

[alexanderivrii]

Thanks, I realized that before I saw your comment :).