Fix handling of globally defined operations in Target

[mtreinish]

Summary

This commit fixes the handling in the target for working with instructions defined globally in the target. It adds handling to the operation_names_for_qargs() and operations_for_qargs() methods to build outputs that include globally defined operations instead of ignoring them. Additionally the add_instruction() method is updated to validate the input qargs to ensure that only valid qubits and qargs are allowed to be added to the target for an instruction.

Details and comments

Fixes #8914

[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

[coveralls]

Pull Request Test Coverage Report for Build 3269585468

• 25 of 25 (100.0%) changed or added relevant lines in 2 files are covered.
• 98 unchanged lines in 5 files lost coverage.
• Overall coverage decreased (-0.1%) to 84.623%

---

Files with Coverage Reduction	New Missed Lines	%
src/optimize_1q_gates.rs	1	95.16%
qiskit/extensions/quantum_initializer/squ.py	2	79.78%
src/sampled_exp_val.rs	14	63.93%
src/results/marginalization.rs	20	77.01%
src/sabre_swap/mod.rs	61	80.45%

Totals
Change from base Build 3269550990:	-0.1%
Covered Lines:	61833
Relevant Lines:	73069

---

💛 - Coveralls

[jakelishman]

On the whole, this looks good to merge to me, and I'm glad it also handles variadic instructions correctly. My only comments are minor questions / gripes that don't actually need any action within this PR.

[jakelishman]

This is probably out-of-scope for this PR, but some minor griping: the logic flow of this function is getting a little tricky for me now, since when I originally read this, I thought it would cause bugs with variadic instructions, as you can't do type.num_qubits. In reality, it all works because properties is required to be {None: None}, so qarg is not None implies (transitively) that instruction.num_qubits should be valid, but because it's in a for qarg in properties loop, that's not so obvious to me any more.

Basically, I'm just griping that it's getting hard to keep all the different assumptions/implications about the objects in my head now. There's nothing actionable unless there's an obvious way to refactor the logic a little now that we've been forced to support variadic arguments.

[mtreinish]

Yeah, I feel like I need to write a big comment about all the different internal states and ways we track things. Part of it just is there are a lot of fiddly details here as there are basically 4 classes of instructions: normal instructions that have qubit restrictions either with or without properties, globally defined ideal instances of operations that have qubit and parameter restrictions, and globally defined variable width instructions. We've basically grafted the last two of those onto the target model which was designed for the first two (the 3rd class was originally just a shorthand to support ideal simulation without adding every permutation of qubit with no properties payload).

In this particular case I had tried to do this check outside the loop but I had logic flow issues with that as we still need to run this loop for the global fixed width instruction case even if this check itself isn't valid for those cases. I think we can look at trying to improve this in a separate refactor in a followup, especially as that I wouldn't want to backport necessarily.

[jakelishman]

Yeah, it's absolutely a follow-up thing, if we do it at all.

[ajavadia]

LGTM