Add qiskit.generic device that accepts instantiated qiskit backends.

[airwoodix]

This patch introduces a new qiskit.generic device that accepts instantiated Qiskit backends and wraps them into a Pennylane device.

This offers a more composable mechanism for specifying the Qiskit backend, especially for providers that require more filter criteria than only a name to select a unique backend with get_backend.

Basic integration tests were added.

A small patch on the call to JobV1.result() was done: changes in #298 pass a timeout argument to JobV1.result(). While supported by many job handles, including that from IBMQ, it is not part of the ABC definition and is therefore not required to be valid.

[codecov]

Codecov Report

Merging #304 (68dcd6a) into master (31d604f) will not change coverage.
The diff coverage is 100.00%.

@@            Coverage Diff            @@
##            master      #304   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            6         7    +1     
  Lines          302       315   +13     
=========================================
+ Hits           302       315   +13     

Impacted Files	Coverage Δ
pennylane_qiskit/__init__.py	100.00% <100.00%> (ø)
pennylane_qiskit/qiskit_device.py	100.00% <100.00%> (ø)
pennylane_qiskit/remote.py	100.00% <100.00%> (ø)

[timmysilv]

thanks for putting this together, it looks great! Device construction feels much more intuitive. just a few things to bring up:

• If you can test the V2 for backend.name and calling job.result() without timeout, it would be sweet. If not, we'll have to skip it somehow. Dealing with the changing IBMQ API can be tough sometimes so it's all good
• Can you give some details on the specific scenario in which you want to pass an already-existing device? It feels a bit funny making a new class just to implement short_name (not too bad, but I guess there are other ways around that if that's the only problem 😋 )

Don't forget to add a changelog entry!

[airwoodix]

Thanks for the positive review!

If you can test the V2 for backend.name and calling job.result() without timeout, it would be sweet. If not, we'll have to skip it somehow. Dealing with the changing IBMQ API can be tough sometimes so it's all good

I'll have a look. It turns out to be difficult to find a concrete implementation of a BackendV2 that doesn't also implement part of the BackendV1 interface. Maybe a dummy would do as well?

Can you give some details on the specific scenario in which you want to pass an already-existing device?

• if a backend needs to be configured (e.g. setting options), it's more convenient to do it before wrapping it as a PennyLane device that retrieving the backend through the private and untyped self._capabilities["backend"] value;
• some providers might require more than just a name string to uniquely identify a backend. This is allowed by ProviderV1.get_backend using the extra keyword arguments. Rather than expanding the qml.device() signature to support this, I think it's easier and more elegant to use dependency injection.

It feels a bit funny making a new class just to implement short_name (not too bad, but I guess there are other ways around that if that's the only problem yum )

I initially intended to use QiskitDevice directly. However, making the provider argument optional (null by default) would require modifying the argument order in QiskitDevice.__init__, which would be a major breaking change.

Don't forget to add a changelog entry!

Done. For the list of contributors, I'd be happy about adding an affiliation. Would that be fine for you?

[timmysilv]

this all makes sense, and looks good! Just one concern with naming. Also, could you update index.rst with a title card for this new device, and have it point to a new file in the devices/ subfolder? It could be small like BasicAer's file, just something to show how/why to use this new device. Thanks!

[timmysilv]

regarding your questions:

• don't worry much about the testing of V2 stuff, it's nearly identical
• those scenarios make sense. In the past, we'd have preferred you make a new device class for the specific device you're talking about, and having it accept the various custom args needed. This is an overall goal of the PennyLane plugins, but I can appreciate that this case goes beyond it (given the provider requirement). Perhaps we'll update the interaction between provider and backend in a later release.
• Changing the arg/kwarg order isn't the best news, but it is essentially half of the point of this PR so it'll do. Plus, it's an isolated change
• the affiliation in the changelog is fine by us!

[airwoodix]

Thanks for the review!

As requested, I renamed qiskit.generic to qiskit.remote and added a (very) short documentation in doc/devices/remote.rst.

I also added explicit #pragma: no cover directives for the backend v1/v2 and timeout/no timeout switches.

@timmysilv from your last comment, I understand that the remaining open threads do not need immediate action. Please let me know if that's not the case.

[timmysilv]

great stuff, thanks for addressing all our concerns! glad to have this in 🎉 and yes, you can disregard the previous comments 😄

[timmysilv]

re: docs failure, I think you just need to add it to the list here

the codecov thing is unrelated, I'm looking into it outside of this PR. don't worry about it - we can merge this in despite that failure

[airwoodix]

Sounds great, thanks a lot for considering this patch and reviewing it.

re: docs failure

Thanks for the hint! Should be fixed now.

[timmysilv]

looks great! I don't have permission to update the branch on your fork, but I fixed the codecov issue on master. If you pull in the latest master, I'll re-run CI and merge 😎

[airwoodix]

Thanks! I rebased on latest master.