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 tape to graph conversion for circuit cutting #2107

Merged
merged 26 commits into from
Jan 31, 2022

Conversation

anthayes92
Copy link
Contributor

Context:
A key aspect of circuit cutting will be to allow users to request automatic cut locations, this relies on graph partitioning algorithms which requires a specific multigraph representation of circuits.

Description of the Change:
Add a tape-to-graph method to the qcut.compiler

Benefits:
Enables conversion of tapes to custom graph representations

Possible Drawbacks:
Some potential overlap with work already done for qml.CircuitGraph (whose representation of wires as edges is not sufficient for graph partitioning needs)

Related GitHub Issues:

@codecov
Copy link

codecov bot commented Jan 18, 2022

Codecov Report

Merging #2107 (43d04a8) into master (d09598c) will increase coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #2107   +/-   ##
=======================================
  Coverage   99.20%   99.20%           
=======================================
  Files         230      231    +1     
  Lines       17718    17745   +27     
=======================================
+ Hits        17577    17604   +27     
  Misses        141      141           
Impacted Files Coverage Δ
pennylane/transforms/qcut.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 d09598c...43d04a8. Read the comment docs.

@anthayes92 anthayes92 changed the title add tape to graph conversion and unit test Add tape to graph conversion for circuit cutting Jan 18, 2022
Base automatically changed from sc-13389-a-user-can-specify-wire-cut-locations-during to master January 20, 2022 00:54
@anthayes92 anthayes92 marked this pull request as ready for review January 21, 2022 19:28
sub_ops = op.expand().expand(depth=1).operations
for sub_op in sub_ops:
add_operator_node(graph, sub_op, order, wire_latest_node)
order += 1
Copy link
Contributor

Choose a reason for hiding this comment

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

Already enumerated, no need to manually increment right? same below.

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 think this can be removed but we need to keep an increment below for when it moves from converting operators to converting measurements and observables


for order, op in enumerate(tape.operations):
if op.name in state_preps:
sub_ops = op.expand().expand(depth=1).operations
Copy link
Contributor

Choose a reason for hiding this comment

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

I do think it makes sense to give users more control when they wish not to do the expansion. But from the automatic cutting perspective, defaulting to doing the expansion makes sense.

wire_latest_node[wire] = op


def tape_to_graph(tape: QuantumTape) -> MultiDiGraph:
Copy link
Contributor Author

@anthayes92 anthayes92 Jan 25, 2022

Choose a reason for hiding this comment

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

Hey @maliasadi this is the tape to graph converter mentioned in the PL meeting. It is currently using networkx, would be great to get your thought on a retworkx implementation.

A couple of thoughts from our side:

  1. We require a MultiDiGraph representation, the rx equivalent seems to be PyDiGraph(..., multigraph=True) but perhaps there is another option?
  2. It's not immediately clear how we can include our custom attributes when adding nodes to a graph in rx, e,g currently we have graph.add_node(m_, order=order)
  3. What are the benefits of using rx over nx? Since we are only converting tapes to graphs the speedup that rx offers doesn't seem to be of much consequence but perhaps we're missing something!

Copy link
Member

Choose a reason for hiding this comment

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

Hey @anthayes92. Thanks for sharing! Checking the code, I don't think that there is anything here nx offers and rx doesn't; however, I agree on this that the performance may be independent of the final choice.

We require a MultiDiGraph representation, the rx equivalent seems to be PyDiGraph(..., multigraph=True) but perhaps there is another option?

You're right! The equivalence of MultiDiGraph is PyDiGraph in rx. You can check circuit_graph.py for example.

It's not immediately clear how we can include our custom attributes when adding nodes to a graph in rx, e,g currently we have graph.add_node(m_, order=order)

In rx, graph attributes can be defined as callback functions (to return statically typed data from node/edge). A good example from here: In nx, we have

import networkx as nx

graph = nx.MultiDiGraph()
graph.add_edges_from([(0, 1, {'weight': 1}), (0, 2, {'weight': 2}),
                      (1, 3, {'weight': 2}), (3, 0, {'weight': 3})])
dist_matrix = nx.floyd_warshall_numpy(graph, weight='weight')

while in rx, you may write it as,

import retworkx as rx

graph = rx.PyDiGraph()
graph.extend_from weighted_edge_list(
    [(0, 1, {'weight': 1}), (0, 2, {'weight': 2}),
     (1, 3, {'weight': 2}), (3, 0, {'weight': 3})])
dist_matrix = rx.digraph_floyd_warshall_numpy(
    graph, weight_fn=lambda edge: edge[weight])

What are the benefits of using rx over nx?

As far as I dig, constructing a graph and updating nodes/edges using the rx Python interface shouldn't add any noticeable performance; however, when it comes to the use of graph algorithms or generators, your rx code will be by far faster and performant than your nx. (check PR #1791 for benchmark)

Copy link
Contributor

Choose a reason for hiding this comment

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

@anthayes92 I could be a bit out of the loop, but this seems interesting. Just wondering what kind of graph algorithms are we looking to run on the qcut DAGs that can benefit from rx, or even nx?

Copy link
Contributor Author

@anthayes92 anthayes92 Jan 26, 2022

Choose a reason for hiding this comment

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

Thanks for the useful insight @maliasadi! It looks like this is certainly possible to do this with rx but as @zeyueN has pointed out, the question is whether it is necessary. Since PL is moving toward rx there is some motivation there however the need for a graph structure that is well suited to graph partitioning algorithms should be the priority here.

@zeyueN will this current conversion of tape to dag be of use for a dedicated dag class? i.e
tape -> nx.dag -> class DAG

If not then perhaps this conversion might only be used for manual circuit cutting.

Copy link
Contributor

Choose a reason for hiding this comment

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

For the DAG class, I was thinking more along the lines of wrapping nx.dag inside it: either by making it an attribute, e.g. self.dag, or, directly inheriting from MultiDiGraph. This way, we can define our own syntax sugars for getting things like list(g.edges(data=True)) as well as manual/auto cut functions. In other words, the tape_to_graph function can be the __init__() of the class.

But ultimately I guess this comes down to how much of these do we want to expose to users, and is having access to the vanilla nx graph dag important to users.

Copy link
Member

Choose a reason for hiding this comment

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

@anthayes92 @zeyueN, a quick flyby comment, but my understanding for building the multi digraph was that two graph algorithms needed to be applied:

  • Graph partitioning
  • Extracting weakly connected graph fragments.

While the graph construction will likely not be any more performant using RX, my understanding is that, once the graph is constructed, applying these two graph algorithms will be much more performant in RX than NX?

Copy link
Member

Choose a reason for hiding this comment

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

Though I'm cautious to put too much work into this while we have an existing CircuitGraph class that wraps nx.DiGraph (not multi). The end state might be the functionality we have here merges with the established PL CircuitGraph.

💯

Copy link
Contributor

Choose a reason for hiding this comment

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

@josh146 The graph partitioning is going to be handled with a more "heavyduty" library kahypar (optional dependency) which uses its own graph representation. But yes the "extracting weakly connected fragments" could be more performant, especially for manually placed cuts like Tom mentioned.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hye @josh146, thanks for weighing in! Yes for the manual cutting case we will need an algorithm to find the weakly connected components (and possibly for the automated cutting case?) and certainly a graph partitioning algo for the automated case.

The big question is whether using rx will give any advantage for graph partitioning - we have been prototyping using kayhpar for this fairly specialised partitioning and not using any rx native algos (in fact I'm not aware of any built in rx graph partitioning algos, though there are some built in to nx).

Since kayhypar expects graphs to be of a specific format, it doesn't seem to make sense to use rx and also motivates creating our own kahypar-friendly DAG class (as Zeyue suggested) for the automated cutting.

Copy link
Member

Choose a reason for hiding this comment

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

Thanks for the useful insight @maliasadi! It looks like this is certainly possible to do this with rx but as @zeyueN has pointed out, the question is whether it is necessary. Since PL is moving toward rx there is some motivation there however the need for a graph structure that is well suited to graph partitioning algorithms should be the priority here.

I want to say if you get an up and running nx implementation, getting rx support should be quite easy as we already dug into this for circuit_graph and qaoa in PL. So, as you mentioned, we can go on with nx if the main use case is to construct/update graphs and talk about this more some other time. 🚀

@zeyueN
Copy link
Contributor

zeyueN commented Jan 26, 2022

After having played with the dags a bit, I think it does have everything I need for auto wire cuts. One minor comment, it feels like having a dedicated dag class could make things a bit easier to work with (instead of working directly with the networkx graph whose interface is a bit awkward, especially when accessing custom attributes).

doc/releases/changelog-dev.md Outdated Show resolved Hide resolved
doc/releases/changelog-dev.md Outdated Show resolved Hide resolved
doc/releases/changelog-dev.md Outdated Show resolved Hide resolved
from pennylane.tape import QuantumTape


def add_operator_node(
Copy link
Contributor

Choose a reason for hiding this comment

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

Could make this _add_operator_node since it looks like more of an internal function

wire_latest_node[wire] = op


def tape_to_graph(tape: QuantumTape) -> MultiDiGraph:
Copy link
Contributor

Choose a reason for hiding this comment

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

The only heavy functionality we'd use from NX/RX is in this line, i.e., pulling apart the graph into its disconnected components. This could also be done as part of the automatic cut placement, but it seemed easier to directly use the NX version (which also accounts for if a user does just manual placement and doesn't want to call the automatic placement workflow).

For the DAG class, I was thinking more along the lines of wrapping nx.dag inside it...

I think that'd be great, especially inheriting from MultiDiGraph. Though I'm cautious to put too much work into this while we have an existing CircuitGraph class that wraps nx.DiGraph (not multi). The end state might be the functionality we have here merges with the established PL CircuitGraph.



def tape_to_graph(tape: QuantumTape) -> MultiDiGraph:
"""Converts a quantum tape to a directed multigraph."""
Copy link
Contributor

Choose a reason for hiding this comment

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

In this PR I think we can leave out adding this function to the transforms docstrings and wait until all of the functionality for circuit cutting is available. However, it'd be good to have the full docstrings added to each function as we go, so that we just need to make them visible in the docstrings without having to write them. So here we should add a docstring for tape_to_graph.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There's a couple of things that have surfaced as "todo when XYZ has been merged". This makes me wonder if there's a good tool for reminders in this scenario. It seems the only GH reminder tool is slack messages for PRs that need reviewing

Comment on lines 44 to 52
wire_latest_node = {w: None for w in tape.wires}
state_preps = ["BasisState", "QubitStateVector"]

for order, op in enumerate(tape.operations):
if op.name in state_preps:
sub_ops = op.expand().expand(depth=1).operations
for sub_op in sub_ops:
add_operator_node(graph, sub_op, order, wire_latest_node)
order += 1
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm in favour of dropping expansion here and having users either: (i) expand the tape themselves before calling this fn, (ii) providing a custom expand_fn when using the cut_circuit transform.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sounds good, I've removed the state prep decomposition and tests

Copy link
Contributor

@trbromley trbromley left a comment

Choose a reason for hiding this comment

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

Thanks @anthayes92 this looks great! Just a few comments/suggestions but once they're sorted this is good to merge.

pennylane/transforms/qcut.py Outdated Show resolved Hide resolved
pennylane/transforms/qcut.py Outdated Show resolved Hide resolved
Comment on lines 69 to 74
graph.add_node(m, order=order)
order += 1

for wire in m.wires:
parent_op = wire_latest_node[wire]
graph.add_edge(parent_op, m, wire=wire)
Copy link
Contributor

Choose a reason for hiding this comment

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

Could we use add_operator_node for this (and lines 63 - 37)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added thanks!

tests/transforms/test_qcut.py Show resolved Hide resolved
Comment on lines 76 to 79
assert (edges[-1][0], edges[-1][1].obs) == (
expected_edge_connections[-1][0],
expected_edge_connections[-1][1],
)
Copy link
Contributor

Choose a reason for hiding this comment

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

🤔 so we have to treat the last edge differently because of MeasurementProcess?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That was the idea however after rethinking it it makes more sense to compare this with tape.measurements[0] from the original tape rather than tape.observables[0]

]

for edge, expected_edge in zip(edges[:-1], expected_edge_connections[:-1]):
assert (edge[0], edge[1]) == (expected_edge[0], expected_edge[1])
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
assert (edge[0], edge[1]) == (expected_edge[0], expected_edge[1])
assert edge == expected_edge

Could this work if we add the label in expected_edge_connections?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

nice, added!

tests/transforms/test_qcut.py Outdated Show resolved Hide resolved
tests/transforms/test_qcut.py Outdated Show resolved Hide resolved
"""

g = qcut.tape_to_graph(tape)
edge_data = list(g.edges(data=True))
Copy link
Contributor

Choose a reason for hiding this comment

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

I think you can also go data="wire" and you'll get just the wire data.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

As in g.edges(data="wire") ? this didn't work for me 🤔 I also tried g.edges.data("wire") with no luck

node_preps = nodes[:2]
for node_prep, expected_prep in zip(node_preps, expected_prep):
assert node_prep.wires == expected_prep.wires
assert node_prep.name == expected_prep.name
Copy link
Contributor

Choose a reason for hiding this comment

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

Could add a test with a tape that contains two measurements. One meas could be a tensor product and the other not.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added thanks!

anthayes92 and others added 2 commits January 27, 2022 14:25
Copy link
Contributor

@trbromley trbromley left a comment

Choose a reason for hiding this comment

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

Thanks @anthayes92!

doc/releases/changelog-dev.md Outdated Show resolved Hide resolved
pennylane/transforms/qcut.py Outdated Show resolved Hide resolved

def _add_operator_node(
graph: MultiDiGraph, op: Operator, order: int, wire_latest_node: dict
) -> None:
Copy link
Contributor

Choose a reason for hiding this comment

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

The return type could be optional in this case 🙂

pennylane/transforms/qcut.py Outdated Show resolved Hide resolved
pennylane/transforms/qcut.py Show resolved Hide resolved
pennylane/transforms/qcut.py Outdated Show resolved Hide resolved
anthayes92 and others added 4 commits January 31, 2022 11:16
Co-authored-by: Tom Bromley <49409390+trbromley@users.noreply.github.com>
Co-authored-by: Tom Bromley <49409390+trbromley@users.noreply.github.com>
@anthayes92 anthayes92 merged commit ced3499 into master Jan 31, 2022
@anthayes92 anthayes92 deleted the sc-13390-a-user-can-convert-a-tape-to-an-accurate branch January 31, 2022 20:08
josh146 pushed a commit that referenced this pull request Feb 4, 2022
* Updated non_parametric ops adjoint method and added test

* changelog

* lint

* fixed testing bug

* lint

* moving updated adjoint logic into the Operation class and override this method in Observable class

* updated black (#2141)

* Add tape to graph conversion for circuit cutting (#2107)

* add WireCut operator, add qcut package, update docs

* add unit test

* update changelog

* updates

* add tape to graph conversion and unit test

* add unit tests

* add obs to node tests

* restructure qcut module

* add tests for observable and measurement conversion to nodes, update stateprep conversion logic

* fix pylint

* Apply suggestions from code review

Co-authored-by: Tom Bromley <49409390+trbromley@users.noreply.github.com>

* add code review suggestions

* add unit tests

* format

* Apply suggestions from code review

Co-authored-by: Tom Bromley <49409390+trbromley@users.noreply.github.com>

* add review suggestions

* Update pennylane/transforms/qcut.py

Co-authored-by: Tom Bromley <49409390+trbromley@users.noreply.github.com>

Co-authored-by: Tom Bromley <49409390+trbromley@users.noreply.github.com>

* add fix and tests (#2145)

* Incrementing the version number to `0.22.0-dev` (#2142)

* clean changelog-dev

* version bump

* mark current and dev release in the changelog

* Add Qchem v0.21.0 release notes

* Over wrote adjoint method for these templates as they inherit from Operations

* implemented adjoint method for a few more templates

* `WireCut` nodes can be replaced with `MeasureNode` and `PrepareNode` (#2124)

* add WireCut operator, add qcut package, update docs

* add unit test

* update changelog

* updates

* add tape to graph conversion and unit test

* add unit tests

* add obs to node tests

* restructure qcut module

* add tests for observable and measurement conversion to nodes, update stateprep conversion logic

* fix pylint

* add method to replace Wirecut ops with Measure and Prepare ops

* add unit tests

* add unit tests, update changelog

* format

* set measure and prepare node order to be equal (remove 0.5)

* Apply suggestions from code review

Co-authored-by: Tom Bromley <49409390+trbromley@users.noreply.github.com>

* add code review suggestions

* add unit tests

* format

* format

* Apply suggestions from code review

Co-authored-by: Tom Bromley <49409390+trbromley@users.noreply.github.com>

* add review suggestions

* Update pennylane/transforms/qcut.py

Co-authored-by: Tom Bromley <49409390+trbromley@users.noreply.github.com>

* Apply suggestions from code review

Co-authored-by: Tom Bromley <49409390+trbromley@users.noreply.github.com>

* add review suggestions

* add unit test

* update changelog

* update changelog-0.21.0

* Apply suggestions from code review

Co-authored-by: Tom Bromley <49409390+trbromley@users.noreply.github.com>

* apply review suggestions

* Apply suggestions from code review

Co-authored-by: Tom Bromley <49409390+trbromley@users.noreply.github.com>

Co-authored-by: Tom Bromley <49409390+trbromley@users.noreply.github.com>

* Update changelog-0.21.0.md (#2149)

Co-authored-by: antalszava <antalszava@gmail.com>

* reset to previous soln

* clean

* more cleaning

* addressing code-review comments

* lint

* remove qcut feature

* reset changelogs

* reset logs

* updated changelog and qchem version

Co-authored-by: Christina Lee <christina@xanadu.ai>
Co-authored-by: anthayes92 <34694788+anthayes92@users.noreply.github.com>
Co-authored-by: Tom Bromley <49409390+trbromley@users.noreply.github.com>
Co-authored-by: antalszava <antalszava@gmail.com>
Co-authored-by: Nathan Killoran <co9olguy@users.noreply.github.com>
josh146 added a commit that referenced this pull request Feb 8, 2022
* Run updated version of black on repo (#2140)

* Run updated black on repo

* Also reformat tests

* Dont convert variables to numpy if on interface-specific device (#2136)

* dont unwrap interface devices if diffmethod None

* tests and black

* dont expect warning on test, update changelog

* readability update and test fix

* lint

* Update doc/releases/changelog-dev.md

Co-authored-by: antalszava <antalszava@gmail.com>

* Modify changelog for psi4 bug fix (#2144)

* modify changelog for psi4 bug fix

* move the entry to the QChem changelog

Co-authored-by: Antal Szava <antalszava@gmail.com>

* Add useful error message on empty batch transform (#2139)

* Add error on empty batch transform

When a batch transform collects an empty `params` list, it currently
raises an IndexError. This fix adds a check to raise a useful error
message in cases were there are no trainable parameters and
`all_operations` was not set to True.

Also fix template argument types in docstrings.

* Add test case

* Update changelog-dev.md

Co-authored-by: Romain <rmoyard@gmail.com>
Co-authored-by: antalszava <antalszava@gmail.com>
Co-authored-by: Josh Izaac <josh146@gmail.com>

* Add warning when using `qml.grad`/`qml.jacobian` without any trainable parameters (#2148)

* Add warning for gradients with no trainable params

* Fix tests expecting different warning

* Fix many tests with no trainable params

Catch expected warnings

* Formatting

* Add changelog

* Update doc/releases/changelog-dev.md

* Use quotes instead of backticks

Co-authored-by: antalszava <antalszava@gmail.com>

* changelog typos

* Add `qml.hf.hamiltonian.simplify` to docs (#2162)

* Allow qml.hf.simplify

* dummy change to trigger CI

* PL version bump

* add 0.21.0 file

* Update (#2167)

* create sections

* org

* update

* correct examples

* Num params fix2 (#2135)

* made num_params a class property in all remaining Operations as it indeed is independen of the instance in all currently defined Operations
Changed Operation class to allow sub-classes to define num_params as instance or class property as desired

* made num_params of Identity static

* added a simple test to check that for some operations whose num_params can be a static class property this is actually the case

* updated changelog

* linting

* linting

* take both static and instance num_params property into account when checking number of provided parameters

* black

* Update pennylane/operation.py

Co-authored-by: Maria Schuld <maria@xanadu.ai>

* Update pennylane/operation.py

Co-authored-by: Maria Schuld <maria@xanadu.ai>

* communting evolution cannot (and thus should not) declare num_params on the class level

* moved num_params test to later in Operation.__init__()
added comments to provide more context
fixed typo

* reverting change in operation.py becaseu they are not needed

* test all ways of declaring num_params

* Update doc/releases/changelog-dev.md

Co-authored-by: Maria Schuld <maria@xanadu.ai>

* removed optional test

Co-authored-by: Maria Schuld <maria@xanadu.ai>

* black

* Update doc/releases/changelog-dev.md

Co-authored-by: Josh Izaac <josh146@gmail.com>
Co-authored-by: Maria Schuld <maria@xanadu.ai>
Co-authored-by: Maria Schuld <mariaschuld@gmail.com>
Co-authored-by: antalszava <antalszava@gmail.com>

* Extended list of contributors

* QChem

* minor

* section names

* Updated non_parametric ops adjoint method and added test (#2133)

* Updated non_parametric ops adjoint method and added test

* changelog

* lint

* fixed testing bug

* lint

* moving updated adjoint logic into the Operation class and override this method in Observable class

* updated black (#2141)

* Add tape to graph conversion for circuit cutting (#2107)

* add WireCut operator, add qcut package, update docs

* add unit test

* update changelog

* updates

* add tape to graph conversion and unit test

* add unit tests

* add obs to node tests

* restructure qcut module

* add tests for observable and measurement conversion to nodes, update stateprep conversion logic

* fix pylint

* Apply suggestions from code review

Co-authored-by: Tom Bromley <49409390+trbromley@users.noreply.github.com>

* add code review suggestions

* add unit tests

* format

* Apply suggestions from code review

Co-authored-by: Tom Bromley <49409390+trbromley@users.noreply.github.com>

* add review suggestions

* Update pennylane/transforms/qcut.py

Co-authored-by: Tom Bromley <49409390+trbromley@users.noreply.github.com>

Co-authored-by: Tom Bromley <49409390+trbromley@users.noreply.github.com>

* add fix and tests (#2145)

* Incrementing the version number to `0.22.0-dev` (#2142)

* clean changelog-dev

* version bump

* mark current and dev release in the changelog

* Add Qchem v0.21.0 release notes

* Over wrote adjoint method for these templates as they inherit from Operations

* implemented adjoint method for a few more templates

* `WireCut` nodes can be replaced with `MeasureNode` and `PrepareNode` (#2124)

* add WireCut operator, add qcut package, update docs

* add unit test

* update changelog

* updates

* add tape to graph conversion and unit test

* add unit tests

* add obs to node tests

* restructure qcut module

* add tests for observable and measurement conversion to nodes, update stateprep conversion logic

* fix pylint

* add method to replace Wirecut ops with Measure and Prepare ops

* add unit tests

* add unit tests, update changelog

* format

* set measure and prepare node order to be equal (remove 0.5)

* Apply suggestions from code review

Co-authored-by: Tom Bromley <49409390+trbromley@users.noreply.github.com>

* add code review suggestions

* add unit tests

* format

* format

* Apply suggestions from code review

Co-authored-by: Tom Bromley <49409390+trbromley@users.noreply.github.com>

* add review suggestions

* Update pennylane/transforms/qcut.py

Co-authored-by: Tom Bromley <49409390+trbromley@users.noreply.github.com>

* Apply suggestions from code review

Co-authored-by: Tom Bromley <49409390+trbromley@users.noreply.github.com>

* add review suggestions

* add unit test

* update changelog

* update changelog-0.21.0

* Apply suggestions from code review

Co-authored-by: Tom Bromley <49409390+trbromley@users.noreply.github.com>

* apply review suggestions

* Apply suggestions from code review

Co-authored-by: Tom Bromley <49409390+trbromley@users.noreply.github.com>

Co-authored-by: Tom Bromley <49409390+trbromley@users.noreply.github.com>

* Update changelog-0.21.0.md (#2149)

Co-authored-by: antalszava <antalszava@gmail.com>

* reset to previous soln

* clean

* more cleaning

* addressing code-review comments

* lint

* remove qcut feature

* reset changelogs

* reset logs

* updated changelog and qchem version

Co-authored-by: Christina Lee <christina@xanadu.ai>
Co-authored-by: anthayes92 <34694788+anthayes92@users.noreply.github.com>
Co-authored-by: Tom Bromley <49409390+trbromley@users.noreply.github.com>
Co-authored-by: antalszava <antalszava@gmail.com>
Co-authored-by: Nathan Killoran <co9olguy@users.noreply.github.com>

* section name and remove imports

* add paper ref

* tapering example

* Update doc/releases/changelog-0.21.0.md

Co-authored-by: Josh Izaac <josh146@gmail.com>

* Update doc/releases/changelog-0.21.0.md

Co-authored-by: Josh Izaac <josh146@gmail.com>

* Update doc/releases/changelog-0.21.0.md

Co-authored-by: Josh Izaac <josh146@gmail.com>

* Update doc/releases/changelog-0.21.0.md

Co-authored-by: Josh Izaac <josh146@gmail.com>

* Update doc/releases/changelog-0.21.0.md

Co-authored-by: Josh Izaac <josh146@gmail.com>

* Breaking changes

* Improvements order

* Update doc/releases/changelog-0.21.0.md

Co-authored-by: Josh Izaac <josh146@gmail.com>

* Update doc/releases/changelog-0.21.0.md

Co-authored-by: Josh Izaac <josh146@gmail.com>

* Update doc/releases/changelog-0.21.0.md

Co-authored-by: Josh Izaac <josh146@gmail.com>

* no well known imports

* apply latest suggestion to the tapering example

* JAX

* Update doc/releases/changelog-0.21.0.md

Co-authored-by: Josh Izaac <josh146@gmail.com>

* Update doc/releases/changelog-0.21.0.md

Co-authored-by: Josh Izaac <josh146@gmail.com>

* Update doc/releases/changelog-0.21.0.md

Co-authored-by: Josh Izaac <josh146@gmail.com>

* trainable weights

* Update doc/releases/changelog-0.21.0.md

Co-authored-by: Josh Izaac <josh146@gmail.com>

* Update doc/releases/changelog-0.21.0.md

Co-authored-by: Josh Izaac <josh146@gmail.com>

* Roto move up

* PR link move up

* Apply suggested headlines

* Apply Rotosolve rephrasing

* emojis

* Fix `dev.num_executions` bug with QNode caching (#2171)

* fix & test

* format

* changelog

* use _tape_cached private attribute on the QNode

* format

* PR number

* changelog

* remove unnecessary comment

* Update doc/releases/changelog-dev.md

Co-authored-by: Josh Izaac <josh146@gmail.com>

Co-authored-by: Josh Izaac <josh146@gmail.com>

* fixed bug when trying to insert around an observable (#2172)

* fixed bug when trying to insert around an observable (or any operation that also inherits from other classes), added tests

* fixed bug in test

* fixing bug in test again

* quick typo fix

* changelog

Co-authored-by: antalszava <antalszava@gmail.com>

* Roto description

* Update doc/releases/changelog-0.21.0.md

Co-authored-by: Josh Izaac <josh146@gmail.com>

* black examples

* Update doc/releases/changelog-0.21.0.md

Co-authored-by: Josh Izaac <josh146@gmail.com>

* remove new lines and suggested sentence

* apply suggestion

* full stop

* Add warnings for the gradient transforms when there are no trainable parameters (#2156)

* Add warnings (tape/qnode) with no trainable params

* Generalize warning + add tests

* Expand warning to all gradient transforms

Also fixes one test warning about shots.

* Add changelog

* code review: warning wording, separate asserts

Co-authored-by: antalszava <antalszava@gmail.com>

* code review: warning wording

* Formatting

* Parametrize qnode tests by interface

* Include finite_difference grad transform

use seperate asserts across tests

* Add import skips for interfaces

Co-authored-by: antalszava <antalszava@gmail.com>

* Deprecate `QubitDevice`'s caching (#2154)

* Add warning and test

* no print

* format

* format

* changelog

* add preferred way of caching

* mark current release

* change an example

* require Lightning v0.21.0 or higher in setup.py

* re-add dev changelog

* `v0.21.0` release notes (#2159)

* changelog typos

* PL version bump

* add 0.21.0 file

* create sections

* org

* update

* correct examples

* Extended list of contributors

* QChem

* minor

* section names

* section name and remove imports

* add paper ref

* tapering example

* Update doc/releases/changelog-0.21.0.md

Co-authored-by: Josh Izaac <josh146@gmail.com>

* Update doc/releases/changelog-0.21.0.md

Co-authored-by: Josh Izaac <josh146@gmail.com>

* Update doc/releases/changelog-0.21.0.md

Co-authored-by: Josh Izaac <josh146@gmail.com>

* Update doc/releases/changelog-0.21.0.md

Co-authored-by: Josh Izaac <josh146@gmail.com>

* Update doc/releases/changelog-0.21.0.md

Co-authored-by: Josh Izaac <josh146@gmail.com>

* Breaking changes

* Improvements order

* Update doc/releases/changelog-0.21.0.md

Co-authored-by: Josh Izaac <josh146@gmail.com>

* Update doc/releases/changelog-0.21.0.md

Co-authored-by: Josh Izaac <josh146@gmail.com>

* Update doc/releases/changelog-0.21.0.md

Co-authored-by: Josh Izaac <josh146@gmail.com>

* no well known imports

* apply latest suggestion to the tapering example

* JAX

* Update doc/releases/changelog-0.21.0.md

Co-authored-by: Josh Izaac <josh146@gmail.com>

* Update doc/releases/changelog-0.21.0.md

Co-authored-by: Josh Izaac <josh146@gmail.com>

* Update doc/releases/changelog-0.21.0.md

Co-authored-by: Josh Izaac <josh146@gmail.com>

* trainable weights

* Update doc/releases/changelog-0.21.0.md

Co-authored-by: Josh Izaac <josh146@gmail.com>

* Update doc/releases/changelog-0.21.0.md

Co-authored-by: Josh Izaac <josh146@gmail.com>

* Roto move up

* PR link move up

* Apply suggested headlines

* Apply Rotosolve rephrasing

* emojis

* Roto description

* Update doc/releases/changelog-0.21.0.md

Co-authored-by: Josh Izaac <josh146@gmail.com>

* black examples

* Update doc/releases/changelog-0.21.0.md

Co-authored-by: Josh Izaac <josh146@gmail.com>

* remove new lines and suggested sentence

* apply suggestion

* full stop

* mark current release

* change an example

Co-authored-by: Josh Izaac <josh146@gmail.com>

* Bump the requirement of PennyLane-Lightning to v0.21.0 in `setup.py` (#2177)

* Bump the requirement of PennyLane-Lightning in setup.py

* Update setup.py

* Update setup.py

Co-authored-by: Josh Izaac <josh146@gmail.com>

* merge

* more

Co-authored-by: David Ittah <dime10@users.noreply.github.com>
Co-authored-by: Christina Lee <christina@xanadu.ai>
Co-authored-by: soranjh <40344468+soranjh@users.noreply.github.com>
Co-authored-by: Romain <rmoyard@gmail.com>
Co-authored-by: Josh Izaac <josh146@gmail.com>
Co-authored-by: cvjjm <60615188+cvjjm@users.noreply.github.com>
Co-authored-by: Maria Schuld <maria@xanadu.ai>
Co-authored-by: Maria Schuld <mariaschuld@gmail.com>
Co-authored-by: Jay Soni <jbsoni@uwaterloo.ca>
Co-authored-by: anthayes92 <34694788+anthayes92@users.noreply.github.com>
Co-authored-by: Tom Bromley <49409390+trbromley@users.noreply.github.com>
Co-authored-by: Nathan Killoran <co9olguy@users.noreply.github.com>
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.

5 participants