Fix handling of ControlledGates in QPY

[mtreinish]

Summary

This commit fixes the handling of ControlledGates in QPY. Previously the
extra parameters needed to reconstruct a custom controlled gate were not
encoded into the QPY payload. Fixing this required a version bump to the
QPY format to modify the payload for a custom instruction entry. Once we
added to the format the extra data required for a controlled gate, the
number of control qubits, the control state, and the base gate object,
the deserializer has enough information to recreate the custom
ControlledGate objects. However, fixing this exposed another bug with
standard library multicontrolled gates where they often didn't contain
sufficient data in the payload to reconstruct either.

Details and comments

Fixes #7999

TODO:

• Fix failing unittest
• Add format docs for QPY v5
• Add release note

[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
• @mtreinish
• @nkanazawa1989

[mtreinish]

I'm not sure that we want to backport this or not, it ended up being a bit more involved than I originally thought it would be.

[coveralls]

Pull Request Test Coverage Report for Build 2518326855

• 90 of 100 (90.0%) changed or added relevant lines in 3 files are covered.
• 11 unchanged lines in 1 file lost coverage.
• Overall coverage decreased (-0.009%) to 84.265%

---

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
qiskit/qpy/binary_io/circuits.py	79	89	88.76%

Files with Coverage Reduction	New Missed Lines	%
qiskit/tools/parallel.py	11	67.69%

Totals
Change from base Build 2518065484:	-0.009%
Covered Lines:	54885
Relevant Lines:	65134

---

💛 - Coveralls

[nkanazawa1989]

Thanks Matthew. Your QPY update looks good to me though I don't quite follow the model of ControlledGate. I have impression that currently developers can too freely define new data model without considering the stability of QPY, and sometime we don't have enough bandwidth to track all changes. This is why I started to finalize the data model of pulse object before adding it.

If circuit model developer will continue to add or modify the data structure, perhaps it would be more sustainable to implement __getstate__ and __setstate__ in all circuit instructions and create QPY binary of it as a free-form dictionary with QPY serialized values?

(EDIT)
Perhaps it doesn't work in terms of backward compatibility of data.

[mtreinish]

Yeah, it's hard to do that from a backwards compatibility standpoint. I also have concerns about how to implement that reliably/securely for arbitrary gate classes. To a certain extent this wasn't actually something new, the data model for circuits is very fractured in general. There are a lot of weird corner cases and classes that behave differently. There is work on trying to standardize it but it's slow moving. In the meantime though we have to deal with all these edge cases.

[nkanazawa1989]

Thanks Matthew. Overall this looks great but I have some small questions.

[nkanazawa1989]

Just curiosity, is it possible to express this with custom_definition_size of zero or non-zero rather than having explicit boolean flag along with size?

[mtreinish]

It is possible, this was mostly an artifact from an earlier version of qpy and really isn't needed. I've just been avoiding changing it because it doesn't hurt and makes another incompatible change.

[nkanazawa1989]

Thanks @mtreinish this looks good to me.