[TEMPLATES] Rewrite QAOAEmbedding and BasicEntanglerLayers #1138
Conversation
Co-authored-by: Chase Roberts <chase@xanadu.ai>
There was a problem hiding this comment.
Thank you @mariaschuld!, the new structure looks good to me.
There was a problem hiding this comment.
This looks great! I much prefer the operation version of the templates.
I left several questions and comments regarding possible improvements. My main concern is that if initializing these templates is difficult for the user, besides incorporating built-in initialization methods, we should also think of ways of making them easier to initialize
| num_wires = AnyWires | ||
| par_domain = "A" | ||
|
|
||
| def __init__(self, weights, wires=None, rotation=None, do_queue=True): |
There was a problem hiding this comment.
I have a question regarding making it easier for users (and devs) to create their own templates. What's the simplest version of a template? To me it seems like all we really need to do is to define an expand method. Other methods are nice but optional. Would you agree?
I'm also curious regarding the difference between expand() and decomposition used for existing operations. Is there a good reason to use both approaches, or is one generally better than the other and should be used throughout?
There was a problem hiding this comment.
Another question, although maybe this was discussed in the ADR. Do we have a plan for supporting custom gradients?
There was a problem hiding this comment.
decompose is the old way to do it, and expand the way forward. In fact, in tape mode expand is called by PL, and expand then uses decompose where necessary to support the old version.
And the important methods are expand but also preprocess where - depending on the template - a lot of stuff can happen. The best example is Motonnen where there is tons of classical pre-processing of the features.
But if a template does not need preprocessing, this method could be left out, it is not a required attribute of the class.
In contrast to before, the only difference really is the init method. If we rewrite Operations it may one day become even easier...
| QUEUES = [ | ||
| (1, (1, 1), ["RX", "RY", "RX"]), | ||
| (2, (1, 3), ["RX", "RX", "MultiRZ", "RY", "RY", "RX", "RX"]), | ||
| ( | ||
| 2, | ||
| (2, 3), | ||
| ["RX", "RX", "MultiRZ", "RY", "RY", "RX", "RX", "MultiRZ", "RY", "RY", "RX", "RX"], | ||
| ), | ||
| ( | ||
| 3, | ||
| (1, 6), | ||
| ["RX", "RX", "RX", "MultiRZ", "MultiRZ", "MultiRZ", "RY", "RY", "RY", "RX", "RX", "RX"], | ||
| ), | ||
| ] |
There was a problem hiding this comment.
Black can be so weird sometimes 🤔
There was a problem hiding this comment.
yes hey? But I thought it is a good opportunity to black the tests...
There was a problem hiding this comment.
PR looks great, just a couple of changes I would like to see before merging:
- Using :meth:
~.QAOAEmbedding.weights_normaland similar to link to methods in the docstrings - Unless someone has objections, a
dimsmethod or similar to make it easier for users to provide their own initialization. I'm still not comfortable with "fixing" the difficuly with initialization by providing built-in initialization methods - There is a
broadcast()on line 150 ofbasic_entangler.pythat shouldn't be there
I'm happy to approve once these changes are made
|
@ixfoduap I changed the weight initialisation methods to return shapes. Let me know if it is good to go! |
…rewrite_qaoaembedding_and_basicentanglerlayers
…ub.com:PennyLaneAI/pennylane into rewrite_qaoaembedding_and_basicentanglerlayers
| from pennylane.init import qaoa_embedding_normal | ||
| weights = qaoa_embedding_normal(n_layers=2, n_wires=2, mean=0, std=0.2) | ||
|
|
||
| shape = QAOAEmbedding.shape(n_layers=2, n_wires=2) |
…ub.com:PennyLaneAI/pennylane into rewrite_qaoaembedding_and_basicentanglerlayers
This is the first in a series of PRs which rewrite the templates as classes that inherit from operations.
The refactor should not change any parts in the code base other than the file containing the template, as well as adding a new file for each template with new tests. For now we keep all old tests, unless they fail for obvious reasons and are clearly replaced by the new tests. When all templates are converted, we can delete the old tests altogether.
Before tackling the remaining templates, we can use this first PR to ion out all problems.