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

Add custom pattern to broadcast #603

Merged
merged 8 commits into from Apr 27, 2020
Merged

Conversation

mariaschuld
Copy link
Contributor

Context:

Adds a custom pattern to the broadcast function. The pattern is specified by passing lists of wires to the function - the unitary is then applied to these wires.

Description of the Change:

  • Adds the custom template functionality
  • Adds example to docstring
  • Add 2 tests
  • Add pattern to documentation

Benefits: This is needed for a new template in the pipeline, and allows users to employ the function in a more flexible manner.

@codecov
Copy link

codecov bot commented Apr 27, 2020

Codecov Report

Merging #603 into master will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master     #603   +/-   ##
=======================================
  Coverage   98.92%   98.92%           
=======================================
  Files          82       82           
  Lines        5099     5109   +10     
=======================================
+ Hits         5044     5054   +10     
  Misses         55       55           
Impacted Files Coverage Δ
pennylane/templates/broadcast.py 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 488bdd7...04a9108. Read the comment docs.

Copy link
Member

@josh146 josh146 left a comment

Choose a reason for hiding this comment

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

Nice thumbnail 😆 Looks great, left a suggestion for an additional unit test

return qml.expval(qml.PauliZ(0))

with pytest.raises(ValueError, match="a custom pattern must be a list"):
circuit()
Copy link
Member

Choose a reason for hiding this comment

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

Nice tests! For a unit test, you could do something that tests that the gates were applied to the correct wires, by using the operation recorder:

def circuit():
    broadcast(unitary=qml.CNOT, pattern=custom_pattern, wires=range(4))

with OperatonRecorder() as rec:
    circuit()

assert rec.queue[0].name == "CNOT"
assert rec.queue[0].wires = custom_pattern[0]

etc.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh yes, I wanted to add that one!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Decided to do one where I check the output instead!

Comment on lines +454 to +456
broadcast(unitary=qml.CNOT, pattern=pattern,
wires=range(5))
return qml.expval(qml.PauliZ(0))
Copy link
Member

Choose a reason for hiding this comment

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

This is really nice! 🔥

@josh146 josh146 added the templates 📁 Updates and additions to the templates label Apr 27, 2020
@mariaschuld mariaschuld changed the title add custom pattern Add custom pattern to broadcast Apr 27, 2020

.. currentmodule:: pennylane.templates

.. autofunction:: broadcast
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This gets automatically added

@@ -36,13 +36,24 @@ State preperations
Custom templates
----------------

Custom templates can be constructed using the template decorator.
The template decorator can used to register a quantum function as a template.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I realised that we did not have the function in the docs...

@@ -30,7 +30,7 @@
import pennylane.init
import pennylane.templates
import pennylane.qnn
from pennylane.templates import template, broadcast
from pennylane.templates import template
Copy link
Contributor Author

Choose a reason for hiding this comment

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

If we want to display broadcast in the API of the templates module, docs complains that there are two functions of the same name, pennylane.broadcast and pennylane.templates.broadcast.

I currently don't remember why we want top-level import of this function, but this would fix it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@josh146 would you know? I have a feeling we discussed this before...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I decided to keep this PR free of the import debate and simply import it in the templates API, but using the qml.broadcast.rst file.

@@ -416,6 +426,8 @@ def circuit(pars):

.. code-block:: python

dev = qml.device('default.qubit', wires=4)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Just added those for consistency.

@mariaschuld
Copy link
Contributor Author

Coverage should not have changed, locally I get 100%.

@mariaschuld mariaschuld merged commit 8be49d9 into master Apr 27, 2020
@mariaschuld mariaschuld deleted the broadcast_custom_pattern branch April 27, 2020 16:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
templates 📁 Updates and additions to the templates
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants