Adds the generator to the PauliRot operation #963

JiahaoYao · 2020-12-14T04:40:20Z

adding the generator for PauliRot Add generators to some operations #895

codecov · 2020-12-14T04:46:23Z

Codecov Report

Merging #963 (4630692) into master (e675ed7) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master     #963   +/-   ##
=======================================
  Coverage   97.82%   97.82%           
=======================================
  Files         150      150           
  Lines       10482    10495   +13     
=======================================
+ Hits        10254    10267   +13     
  Misses        228      228

Impacted Files	Coverage Δ
pennylane/ops/qubit.py	`97.11% <100.00%> (+0.06%)`	⬆️

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 e675ed7...4630692. Read the comment docs.

JiahaoYao · 2020-12-14T04:58:37Z

How can i fix the black?

josh146 · 2020-12-14T07:58:49Z

Thanks @JiahaoYao for the contribution!

Regarding black, have you installed the black formatter (pip install black) and run it on the PL folder with a line length of 100?

black -l 100 pennylane/

If you have, you may need to upgrade your black version and run it again:

pip install black --upgrade
black -l 100 pennylane/

JiahaoYao · 2020-12-14T18:35:31Z

thanks @josh146

co9olguy · 2020-12-16T19:32:58Z

Hi @JiahaoYao, thanks for the contribution! Are you ready for this one to be reviewed?

JiahaoYao · 2020-12-16T19:46:20Z

please @co9olguy

trbromley

Thanks @JiahaoYao, will be nice to add this in! I just left a few comments, but will be happy to approve once they are sorted.

Also, don't forget to add the change and your name to the change log.

tests/ops/test_qubit_ops.py

trbromley · 2020-12-18T00:23:26Z

tests/ops/test_qubit_ops.py

+
+        if pauli_word[0] == 'I':
+            # this is the identity
+            expected_gen = qml.PauliZ(wires=0) @ qml.PauliZ(wires=0)


Suggested change

expected_gen = qml.PauliZ(wires=0) @ qml.PauliZ(wires=0)

expected_gen = qml.Identity(wires=0)

would this work?

trbromley · 2020-12-18T00:25:36Z

tests/ops/test_qubit_ops.py

+                expected_gen = expected_gen @  qml.PauliZ(
+                    wires=i) @  qml.PauliZ(wires=i)


Suggested change

expected_gen = expected_gen @ qml.PauliZ(

wires=i) @ qml.PauliZ(wires=i)

expected_gen = expected_gen @ qml.Identity(wires=i)

cool, i just want this operator.

trbromley · 2020-12-18T00:29:08Z

tests/ops/test_qubit_ops.py

-        spy = mocker.spy(qml.utils, "pauli_eigs")
-
-        op.generator
-        spy.assert_not_called()
-
-


What is the motivation for removing this part? I think it's not strictly required in terms of test coverage, but was a nice addition to check that the generator is not reconstructed each time it is accessed.

i see, previously when i saw this op.generator, I thought this was something that was forgotten to remove. no problem!

JiahaoYao · 2020-12-18T02:50:45Z

Thanks @JiahaoYao, will be nice to add this in! I just left a few comments, but will be happy to approve once they are sorted.

Also, don't forget to add the change and your name to the change log.

Sorry, by the way, how can I add the name to the change log?

JiahaoYao · 2020-12-18T04:43:44Z

changed

josh146

Thanks for this useful addition @JiahaoYao! There is one pending suggestion from @trbromley, but this looks good from my end once that is addressed.

Oh, and don't forget to update the changelog (.github/CHANGELOG.md) with

A short description of the change
A link back to this PR
Your name in the contributors section!

josh146 · 2020-12-18T06:07:57Z

pennylane/ops/qubit.py

+
+            # Simplest case is if the Pauli is the identity matrix
+            if pauli_word == "I" * len(pauli_word):
+                self._generator = [np.eye(2 ** len(pauli_word)), -1 / 2]


@trbromley: we should overhaul how generators work. Rather than returning an operation class or a matrix, we should instead allow returning an operation instance or a matrix.

That way we could support returning qml.Identity(wires=self.wires) here, which avoids creating a large identity matrix.

@josh146, totally agree!

I'll turn this into a GitHub issue 👍

josh146 · 2020-12-18T06:09:10Z

pennylane/ops/qubit.py

+            # We first generate the matrix excluding the identity parts and expand it afterwards.
+            # To this end, we have to store on which wires the non-identity parts act


Really helpful comments!

JiahaoYao · 2020-12-18T07:08:06Z

@josh146 cool, added!

trbromley

Thanks @JiahaoYao, approved! Although I left one comment on the changelog.

trbromley · 2020-12-18T22:27:10Z

.github/CHANGELOG.md

+* Adds the following generators to the `qml.operation` module: `MultiRZ`, `PauliRot`. These 
+  generators are required to construct the metric tensor for some optimizer. 
+  [(#895)](https://github.com/PennyLaneAI/pennylane/pull/895)


Suggested change

* Adds the following generators to the `qml.operation` module: `MultiRZ`, `PauliRot`. These

generators are required to construct the metric tensor for some optimizer.

[(#895)](https://github.com/PennyLaneAI/pennylane/pull/895)

* Adds the `PauliRot` generator to the `qml.operation` module. This

generator is required to construct the metric tensor.

[(#963)](https://github.com/PennyLaneAI/pennylane/pull/963)

Thanks! Removing the mention of MultiRZ since we added a line for that in the previous release.

trbromley · 2020-12-18T22:27:51Z

pennylane/ops/qubit.py

+
+            # Simplest case is if the Pauli is the identity matrix
+            if pauli_word == "I" * len(pauli_word):
+                self._generator = [np.eye(2 ** len(pauli_word)), -1 / 2]


@josh146, totally agree!

tests/ops/test_qubit_ops.py

fixes PennyLaneAI#895

fa12dfd

black format

c939160

josh146 changed the title ~~fixes #895~~ Adds the generator to the PauliRot operation Dec 14, 2020

update black

f5b4ee1

Merge branch 'master' into master

229481c

co9olguy requested review from josh146 and trbromley December 16, 2020 19:51

co9olguy added the review-ready 👌 PRs which are ready for review by someone from the core team. label Dec 16, 2020

Merge branch 'master' into master

d4daae1

trbromley reviewed Dec 18, 2020

View reviewed changes

trbromley mentioned this pull request Dec 18, 2020

Add generators to some operations #895

Closed

4 tasks

JiahaoYao added 2 commits December 17, 2020 20:42

nit

65c8ee1

Merge branch 'master' of github.com:JiahaoYao/pennylane

b2a1514

josh146 approved these changes Dec 18, 2020

View reviewed changes

changelog

e792fd5

trbromley self-requested a review December 18, 2020 14:22

trbromley approved these changes Dec 18, 2020

View reviewed changes

trbromley and others added 3 commits December 18, 2020 17:29

Merge branch 'master' into master

ff0e38e

nit

0a75d95

Merge branch 'master' of github.com:JiahaoYao/pennylane

4630692

josh146 mentioned this pull request Dec 19, 2020

Re-think operation generators #970

Closed

josh146 merged commit bd61abd into PennyLaneAI:master Dec 19, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adds the generator to the PauliRot operation #963

Adds the generator to the PauliRot operation #963

JiahaoYao commented Dec 14, 2020 •

edited

Loading

codecov bot commented Dec 14, 2020 •

edited

Loading

JiahaoYao commented Dec 14, 2020

josh146 commented Dec 14, 2020

JiahaoYao commented Dec 14, 2020

co9olguy commented Dec 16, 2020

JiahaoYao commented Dec 16, 2020

trbromley left a comment

trbromley Dec 18, 2020

JiahaoYao Dec 18, 2020

trbromley Dec 18, 2020

JiahaoYao Dec 18, 2020

trbromley Dec 18, 2020

JiahaoYao Dec 18, 2020

JiahaoYao commented Dec 18, 2020

JiahaoYao commented Dec 18, 2020

josh146 left a comment •

edited

Loading

josh146 Dec 18, 2020

trbromley Dec 18, 2020

josh146 Dec 19, 2020

josh146 Dec 18, 2020

JiahaoYao Dec 18, 2020

JiahaoYao commented Dec 18, 2020

trbromley left a comment

trbromley Dec 18, 2020

JiahaoYao Dec 18, 2020

trbromley Dec 18, 2020

	expected_gen = qml.PauliZ(wires=0) @ qml.PauliZ(wires=0)
	expected_gen = qml.Identity(wires=0)

		expected_gen = expected_gen @ qml.PauliZ(
		wires=i) @ qml.PauliZ(wires=i)

	expected_gen = expected_gen @ qml.PauliZ(
	wires=i) @ qml.PauliZ(wires=i)
	expected_gen = expected_gen @ qml.Identity(wires=i)

		# We first generate the matrix excluding the identity parts and expand it afterwards.
		# To this end, we have to store on which wires the non-identity parts act

Adds the generator to the PauliRot operation #963

Adds the generator to the PauliRot operation #963

Conversation

JiahaoYao commented Dec 14, 2020 • edited Loading

codecov bot commented Dec 14, 2020 • edited Loading

Codecov Report

JiahaoYao commented Dec 14, 2020

josh146 commented Dec 14, 2020

JiahaoYao commented Dec 14, 2020

co9olguy commented Dec 16, 2020

JiahaoYao commented Dec 16, 2020

trbromley left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JiahaoYao commented Dec 18, 2020

JiahaoYao commented Dec 18, 2020

josh146 left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JiahaoYao commented Dec 18, 2020

trbromley left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JiahaoYao commented Dec 14, 2020 •

edited

Loading

codecov bot commented Dec 14, 2020 •

edited

Loading

josh146 left a comment •

edited

Loading