Allow mcrx, mcry and mcrz gate methods to accept angles of ParameterValueType

[ShellyGarion]

Summary

Address #12863

There are currently several open issues that fail with errors like:
QiskitError: 'Cannot synthesize MCRY with unbound parameter:

#7975
#11142
#13192

This PR aims to fix them

Details and comments

[qiskit-bot]

One or more of the following people are relevant to this code:

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

[ShellyGarion]

The failing tests were added as part of #12752.
Perhaps they could be removed?

[Cryoris]

This looks great, I'm glad that we can support parameterized multicontrolled rotations!

I'm wondering whether we can consolidate the method you added with the existing _mcsu2_real_diagonal to ensure we don't duplicate the logic. One option could be to take the unitary as Gate instead of np.ndarray:

def _mcsu2(gate: Gate, num_controls: int, use_basis_gates: bool) -> QuantumCircuit:
    if isinstance(gate, RYGate):
        s_gate = # ...
    elif # the other Pauli rotation cases, RX and RZ
    else:
       unitary = gate.to_matrix()
       s_gate = # compute s_gate
       
   # common logic here

do you think that might work?

[Cryoris]

The failing tests were added as part of #12752.
Perhaps they could be removed?

You refer to test_mc_failure_without_annotation? We can remove RX/RY/RZ in that test, but we likely want to keep the others, I'd think

[ShellyGarion]

The failing tests were added as part of #12752.
Perhaps they could be removed?

You refer to test_mc_failure_without_annotation? We can remove RX/RY/RZ in that test, but we likely want to keep the others, I'd think

These are the failing tests, but all of them are failing (not only for RX / RY / RZ)

[coveralls]

Pull Request Test Coverage Report for Build 12653458685

Warning: This coverage report may be inaccurate.

This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.

• For more information on this, see Tracking coverage changes with pull request builds.
• To avoid this issue with future PRs, see these Recommended CI Configurations.
• For a quick fix, rebase this PR at GitHub. Your next report should be accurate.

Details

• 161 of 189 (85.19%) changed or added relevant lines in 8 files are covered.
• 153 unchanged lines in 12 files lost coverage.
• Overall coverage decreased (-0.02%) to 88.927%

---

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
qiskit/circuit/quantumcircuit.py	67	71	94.37%
qiskit/synthesis/multi_controlled/multi_control_rotation_gates.py	80	104	76.92%

Files with Coverage Reduction	New Missed Lines	%
qiskit/circuit/library/quantum_volume.py	1	93.18%
crates/qasm2/src/expr.rs	1	94.02%
qiskit/circuit/library/arithmetic/adders/adder.py	2	91.49%
qiskit/circuit/add_control.py	4	96.49%
crates/qasm2/src/lex.rs	5	91.23%
qiskit/primitives/containers/observables_array.py	5	95.0%
crates/qasm2/src/parse.rs	6	97.62%
qiskit/quantum_info/states/stabilizerstate.py	11	93.89%
crates/circuit/src/circuit_data.rs	19	95.43%
qiskit/quantum_info/operators/symplectic/sparse_pauli_op.py	20	94.58%

Totals
Change from base Build 12131777933:	-0.02%
Covered Lines:	79423
Relevant Lines:	89313

---

💛 - Coveralls

[Cryoris]

This is not something to fix in this PR: but do you know why we have this special case only for MCRY and not for the others? If it's more efficient we could also use it there.

This is something we can fix very cleanly if we synthesize controlled gates via HLS, which knows the number of free auxiliaries.

[ShellyGarion]

The use of ancillas was done in a PR that you implemented #4787 following issue #4786. I'm not sure if there is some improvement anymore.

[ShellyGarion]

after checking it seems that adding ancillas can improve mcry in certain cases (num_controls >= 7). This is an optimization that should be explored in a follow-up work.

[Cryoris]

One small comment on the docs and one question that doesn't need to be resolved in this PR, otherwise LGTM 👍🏻

[Cryoris]

With @alexanderivrii's PR on the annotated gate plugins, could we change this method in future to just append an annotated Pauli rotation? It would be nice if we can move this logic to the plugin so it doesn't need to be duplicated for the annotated rotation.

[ShellyGarion]

Indeed. I think this could be done in a follow-up PR.

[Cryoris]

Thanks for the improvement! I'm not 100% sure whether this should be a bugfix or a new feature -- do you remember whether parameterized rotations were supported before the changes to mcsu2_real_diag etc. ?

[alexanderivrii]

This looks good on my side as well. For posteriority, could you please add the number of 2-qubit gates we get for multiply-controlled parametric p gates before and after this PR (since we now use the same presumably better algorithm both for parametric and non-parametric gates)? It would be also nice to record somewhere further follow-ups discussed in this PR.