Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Plugin extra operations #65

Closed
wants to merge 15 commits into from
Closed

Plugin extra operations #65

wants to merge 15 commits into from

Conversation

cgogolin
Copy link
Contributor

@cgogolin cgogolin commented Oct 5, 2018

Allows plugins to supply additional Operations and Expectations, which are then automatically accessible along the standard OpenQML Operations form the openqml. and openqml.expectation. contexts.

The device keeps track of which operations it adds and removes them again when the qfunc ends.

This PR in particular solves #44.

The changes are slightly more extensive than I would have wanted, but nothing dramatic. You will see that if you first merge #64:

  • Adding an removing operations is done in device.__enter__() and device.__exit__(), which are now called again because I have reintroduced a with device: into the qfunc decorator. (@josh146, please comment on whether this will break other things)

  • I had to move QuantumFunctionError to qnode, where it anyway belongs more naturally, to resolve a dependency loop.

…ions

disabled AllPauliZ Expectation
fixed generation of DeviceError
fixed calculation of expectation values in the simulator
…but which makes no sense

commented out Identity gate in default plugin which seems useless
added check of whether the required wires fit on the device
…itional operations

small improvements in docstring of qfunc.py
moved QuantumFunctionError from device to qnode
moved code to add/remove the extra operations to __enter__() and __exit__()
@@ -110,6 +110,7 @@ class ProjectQDevice(Device):
plugin_version = __version__
author = 'Christian Gogolin'
_capabilities = {'backend': list(["Simulator", "ClassicalSimulator", "IBMBackend"])}
_extra_operations = {'S': S, 'T': T, 'SqrtX': SqrtX, 'SqrtSwap': SqrtSwap, 'AllPauliZ': AllPauliZ}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So is _extra_operations a optional class attribute, that maps openqml.ops.key (attribute loaded dynamically) to openqml_pq.ops.value?

I'll resolve this if it is explained further down, just commenting here while it is fresh in my mind :)

@@ -21,7 +21,8 @@


from .configuration import Configuration
from .device import Device, DeviceError, QuantumFunctionError
from .device import Device, DeviceError
from .qnode import QuantumFunctionError
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Strangely, I think I had to originally put QuantumFunctionError in device.py because of a dependency loop...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is no longer used in device.py, so I think now it makes more sense to have it in qnode.py

openqml/plugins/default.py Outdated Show resolved Hide resolved
@josh146
Copy link
Member

josh146 commented Oct 5, 2018

It looks good in the case of the qfunc decorator! The only downside I can think of is that in the more advance use-case, where the user instatiates the QNode object directly, i.e.

def quantum_function():
   qm.RX()
   qm.custom_op()

qnode = QNode(quantum_function, dev1)

will not work without an explicit with dev1: prior to the QNode call.

One solution would be to move the with device: to inside the QNode, since the device is passed to the QNode anyway. That way, the explicit with device: in the example above, and also inside qfunc.py, are no longer needed. In fact, this is what I think was happening originally, back when device was still a context manager? Going through the commit history of master should show this.

@cgogolin
Copy link
Contributor Author

cgogolin commented Oct 5, 2018 via email

@cgogolin
Copy link
Contributor Author

cgogolin commented Oct 5, 2018 via email

@cgogolin
Copy link
Contributor Author

cgogolin commented Oct 5, 2018 via email

@cgogolin
Copy link
Contributor Author

cgogolin commented Oct 5, 2018 via email

…and __exit__()

call reset() manually during init
removed already commented Identity operation from default plugin
@cgogolin
Copy link
Contributor Author

cgogolin commented Oct 8, 2018

Replying via email to your comments aparently just put all my replies into this thread instead of treating them as replies to your questions...

With the latest commits I have:

  • added comments to __enter__() and __exit__()

  • moved the device context from the qfunc decorator to QNode. This is clearly the cleaner solution, as you outlined above.

@cgogolin
Copy link
Contributor Author

cgogolin commented Oct 8, 2018

I just finished the classical simulator (I know that this will be of limited use, but it is still nice to support and wasn't much work). With this I think this PR is ready for merging.

@co9olguy
Copy link
Member

co9olguy commented Nov 1, 2018

I'm going to close this PR at this point. We have not come to an agreement on the best way to handle plugins importing their own ops. For initial release, we will go with a simple exposure via the plugin's namespace. We can revisit this question for future releases.

@co9olguy co9olguy closed this Nov 1, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants