From ec9d121a6067af28cd3be66b164a60a7c4561571 Mon Sep 17 00:00:00 2001 From: andrijapau Date: Tue, 22 Oct 2024 12:42:32 -0400 Subject: [PATCH 01/12] parameter-shift matches backprop --- pennylane/ops/op_math/exp.py | 1 + pennylane/templates/subroutines/trotter.py | 2 ++ tests/templates/test_subroutines/test_trotter.py | 1 - 3 files changed, 3 insertions(+), 1 deletion(-) diff --git a/pennylane/ops/op_math/exp.py b/pennylane/ops/op_math/exp.py index 106f387b1e2..9fa30022146 100644 --- a/pennylane/ops/op_math/exp.py +++ b/pennylane/ops/op_math/exp.py @@ -164,6 +164,7 @@ class Exp(ScalarSymbolicOp, Operation): """ + grad_method = None control_wires = Wires([]) _name = "Exp" diff --git a/pennylane/templates/subroutines/trotter.py b/pennylane/templates/subroutines/trotter.py index 87dfac458d6..78691aa128e 100644 --- a/pennylane/templates/subroutines/trotter.py +++ b/pennylane/templates/subroutines/trotter.py @@ -186,6 +186,8 @@ def my_circ(c1, c2, time): (tensor(0.00961064, requires_grad=True), tensor(-0.12338274, requires_grad=True), tensor(-5.43401259, requires_grad=True)) """ + grad_method = None + @classmethod def _primitive_bind_call(cls, *args, **kwargs): # accepts no wires, so bypasses the wire processing. diff --git a/tests/templates/test_subroutines/test_trotter.py b/tests/templates/test_subroutines/test_trotter.py index ce9cd3f1760..e4142bd1e6c 100644 --- a/tests/templates/test_subroutines/test_trotter.py +++ b/tests/templates/test_subroutines/test_trotter.py @@ -447,7 +447,6 @@ def test_copy(self, hamiltonian, time, n, order): assert op.hyperparameters == new_op.hyperparameters assert op is not new_op - @pytest.mark.xfail(reason="https://github.com/PennyLaneAI/pennylane/issues/6333", strict=False) @pytest.mark.parametrize("hamiltonian", test_hamiltonians) def test_standard_validity(self, hamiltonian): """Test standard validity criteria using assert_valid.""" From 96bc0a0d00754f187a3aa1b60b0d13a0a377217d Mon Sep 17 00:00:00 2001 From: andrijapau Date: Tue, 22 Oct 2024 12:48:32 -0400 Subject: [PATCH 02/12] add changelog entry --- doc/releases/changelog-dev.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/doc/releases/changelog-dev.md b/doc/releases/changelog-dev.md index 07066fd8810..498a10c4e70 100644 --- a/doc/releases/changelog-dev.md +++ b/doc/releases/changelog-dev.md @@ -295,6 +295,9 @@

Bug fixes 🐛

+* Fixes incorrect differentiation of `TrotterProduct` when using `diff_method="parameter-shift"`. + [(#6432)](https://github.com/PennyLaneAI/pennylane/pull/6432) + * `default.tensor` can now handle mid circuit measurements via the deferred measurement principle. [(#6408)](https://github.com/PennyLaneAI/pennylane/pull/6408) From 2aa4d7f69f1ffbc2b5bddf4ad738dec1856ca729 Mon Sep 17 00:00:00 2001 From: andrijapau Date: Wed, 23 Oct 2024 15:18:39 -0400 Subject: [PATCH 03/12] skip diff for test case that is not unitary --- tests/ops/functions/conftest.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/ops/functions/conftest.py b/tests/ops/functions/conftest.py index 49a0dd02e8a..23eeaa43a38 100644 --- a/tests/ops/functions/conftest.py +++ b/tests/ops/functions/conftest.py @@ -57,7 +57,7 @@ (qml.s_prod(1.1, qml.RX(1.1, 0)), {}), (qml.prod(qml.PauliX(0), qml.PauliY(1), qml.PauliZ(0)), {}), (qml.ctrl(qml.RX(1.1, 0), 1), {}), - (qml.exp(qml.PauliX(0), 1.1), {}), + (qml.exp(qml.PauliX(0), 1.1), {"skip_differentiation": True}), (qml.pow(qml.IsingXX(1.1, [0, 1]), 2.5), {}), (qml.ops.Evolution(qml.PauliX(0), 5.2), {}), (qml.QutritBasisState([1, 2, 0], wires=[0, 1, 2]), {"skip_differentiation": True}), From 60518ed4cf451a9583bd0ca64d7415e614696e0b Mon Sep 17 00:00:00 2001 From: Andrija Paurevic <46359773+andrijapau@users.noreply.github.com> Date: Fri, 25 Oct 2024 11:43:23 -0400 Subject: [PATCH 04/12] Update tests/ops/functions/conftest.py Co-authored-by: Christina Lee --- tests/ops/functions/conftest.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/ops/functions/conftest.py b/tests/ops/functions/conftest.py index 23eeaa43a38..50d04426e37 100644 --- a/tests/ops/functions/conftest.py +++ b/tests/ops/functions/conftest.py @@ -58,6 +58,8 @@ (qml.prod(qml.PauliX(0), qml.PauliY(1), qml.PauliZ(0)), {}), (qml.ctrl(qml.RX(1.1, 0), 1), {}), (qml.exp(qml.PauliX(0), 1.1), {"skip_differentiation": True}), + (qml.exp(qml.PauliX(0), 2.9j)), + (qml.evolve(qml.PauliX(0), -0.5)), (qml.pow(qml.IsingXX(1.1, [0, 1]), 2.5), {}), (qml.ops.Evolution(qml.PauliX(0), 5.2), {}), (qml.QutritBasisState([1, 2, 0], wires=[0, 1, 2]), {"skip_differentiation": True}), From 8d095d7653786c1ded5f92dd8157e92bcfb80dbb Mon Sep 17 00:00:00 2001 From: andrijapau Date: Fri, 25 Oct 2024 13:52:27 -0400 Subject: [PATCH 05/12] typo in committed suggestion --- tests/ops/functions/conftest.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/ops/functions/conftest.py b/tests/ops/functions/conftest.py index 50d04426e37..27257c079fa 100644 --- a/tests/ops/functions/conftest.py +++ b/tests/ops/functions/conftest.py @@ -58,8 +58,8 @@ (qml.prod(qml.PauliX(0), qml.PauliY(1), qml.PauliZ(0)), {}), (qml.ctrl(qml.RX(1.1, 0), 1), {}), (qml.exp(qml.PauliX(0), 1.1), {"skip_differentiation": True}), - (qml.exp(qml.PauliX(0), 2.9j)), - (qml.evolve(qml.PauliX(0), -0.5)), + (qml.exp(qml.PauliX(0), 2.9j), {}), + (qml.evolve(qml.PauliX(0), -0.5), {}), (qml.pow(qml.IsingXX(1.1, [0, 1]), 2.5), {}), (qml.ops.Evolution(qml.PauliX(0), 5.2), {}), (qml.QutritBasisState([1, 2, 0], wires=[0, 1, 2]), {"skip_differentiation": True}), From 03853c5865511e5cb2a672cf32ee188ceae7573c Mon Sep 17 00:00:00 2001 From: andrijapau Date: Wed, 30 Oct 2024 17:18:34 -0400 Subject: [PATCH 06/12] skip diff and add seperate custom diff check --- .../test_subroutines/test_trotter.py | 37 ++++++++++++++++++- 1 file changed, 36 insertions(+), 1 deletion(-) diff --git a/tests/templates/test_subroutines/test_trotter.py b/tests/templates/test_subroutines/test_trotter.py index e4142bd1e6c..32788a5b514 100644 --- a/tests/templates/test_subroutines/test_trotter.py +++ b/tests/templates/test_subroutines/test_trotter.py @@ -452,7 +452,42 @@ def test_standard_validity(self, hamiltonian): """Test standard validity criteria using assert_valid.""" time, n, order = (4.2, 10, 4) op = qml.TrotterProduct(hamiltonian, time, n=n, order=order) - qml.ops.functions.assert_valid(op) + qml.ops.functions.assert_valid(op, skip_differentiation=True) + + @pytest.mark.parametrize("hamiltonian", test_hamiltonians) + def test_differentiation(self, hamiltonian): + """Tests the differentiation of the TrotterProduct with parameter-shift""" + + time, n, order = (4.2, 10, 4) + + dev = qml.device("default.qubit") + time = qml.numpy.array(time) + coeffs, _ = hamiltonian.terms() + + # FIXME: setting private attribute `_coeffs` as work around + @qml.qnode(dev, diff_method="backprop") + def circ_bp(coeffs, time): + hamiltonian._coeffs = coeffs + qml.TrotterProduct(hamiltonian, time, n, order) + return qml.probs() + + @qml.qnode(dev, diff_method="parameter-shift") + def circ_ps(coeffs, time): + hamiltonian._coeffs = coeffs + qml.TrotterProduct(hamiltonian, time, n, order) + return qml.probs() + + expected_bp = qml.jacobian(circ_bp)(coeffs, time) + ps = qml.jacobian(circ_ps)(coeffs, time) + + error_msg = ( + "Parameter-shift does not produce the same Jacobian as with backpropagation. " + "This might be a bug, or it might be expected due to the mathematical nature " + "of backpropagation, in which case, this test can be skipped for this operator." + ) + + for actual, expected in zip(ps, expected_bp): + assert qml.math.allclose(actual, expected), error_msg # TODO: Remove test when we deprecate ApproxTimeEvolution @pytest.mark.parametrize("n", (1, 2, 5, 10)) From c47adbb8eb513df59c22b1e2fd030a5b9b3b014c Mon Sep 17 00:00:00 2001 From: andrijapau Date: Thu, 31 Oct 2024 10:06:21 -0400 Subject: [PATCH 07/12] comment out failing test case --- tests/ops/functions/conftest.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/ops/functions/conftest.py b/tests/ops/functions/conftest.py index 27257c079fa..35c5b655e8f 100644 --- a/tests/ops/functions/conftest.py +++ b/tests/ops/functions/conftest.py @@ -58,7 +58,8 @@ (qml.prod(qml.PauliX(0), qml.PauliY(1), qml.PauliZ(0)), {}), (qml.ctrl(qml.RX(1.1, 0), 1), {}), (qml.exp(qml.PauliX(0), 1.1), {"skip_differentiation": True}), - (qml.exp(qml.PauliX(0), 2.9j), {}), + # FIXME: Generator of Exp is incorrect when coefficient is imaginary + # (qml.exp(qml.PauliX(0), 2.9j), {}), (qml.evolve(qml.PauliX(0), -0.5), {}), (qml.pow(qml.IsingXX(1.1, [0, 1]), 2.5), {}), (qml.ops.Evolution(qml.PauliX(0), 5.2), {}), From f0753cd1b4c9da0882dba8344f2b32d9f9b5f7a1 Mon Sep 17 00:00:00 2001 From: andrijapau Date: Thu, 2 Jan 2025 13:11:19 -0500 Subject: [PATCH 08/12] fix: remove xfail from test_exp.py --- tests/ops/op_math/test_exp.py | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/ops/op_math/test_exp.py b/tests/ops/op_math/test_exp.py index 5abead84595..c995d6b1dbd 100644 --- a/tests/ops/op_math/test_exp.py +++ b/tests/ops/op_math/test_exp.py @@ -870,7 +870,6 @@ def circuit(phi): grad = qml.grad(circuit)(phi) assert qml.math.allclose(grad, -qml.numpy.sin(phi)) - @pytest.mark.xfail # related to #6333 @pytest.mark.autograd def test_autograd_param_shift_qnode(self): """Test execution and gradient with pennylane numpy array.""" From b005e6a85d32e648c8ceb728218df4d0c6de0684 Mon Sep 17 00:00:00 2001 From: andrijapau Date: Thu, 2 Jan 2025 13:40:26 -0500 Subject: [PATCH 09/12] fix: Update test_differentiation to not use private _coeffs --- tests/templates/test_subroutines/test_trotter.py | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/tests/templates/test_subroutines/test_trotter.py b/tests/templates/test_subroutines/test_trotter.py index 5a4e90adfbc..d39bf0f22ba 100644 --- a/tests/templates/test_subroutines/test_trotter.py +++ b/tests/templates/test_subroutines/test_trotter.py @@ -467,18 +467,21 @@ def test_differentiation(self, hamiltonian): dev = qml.device("default.qubit") time = qml.numpy.array(time) - coeffs, _ = hamiltonian.terms() + coeffs, ops = hamiltonian.terms() - # FIXME: setting private attribute `_coeffs` as work around @qml.qnode(dev, diff_method="backprop") def circ_bp(coeffs, time): - hamiltonian._coeffs = coeffs + with qml.queuing.QueuingManager.stop_recording(): + hamiltonian = qml.dot(coeffs, ops) + qml.TrotterProduct(hamiltonian, time, n, order) return qml.probs() @qml.qnode(dev, diff_method="parameter-shift") def circ_ps(coeffs, time): - hamiltonian._coeffs = coeffs + with qml.queuing.QueuingManager.stop_recording(): + hamiltonian = qml.dot(coeffs, ops) + qml.TrotterProduct(hamiltonian, time, n, order) return qml.probs() From ea533bda5d1b1a7d2b5ac518b3f80d3a119bba7e Mon Sep 17 00:00:00 2001 From: andrijapau Date: Thu, 2 Jan 2025 16:01:36 -0500 Subject: [PATCH 10/12] fix: Remove grad_method=None addition from trotter.py --- pennylane/templates/subroutines/trotter.py | 2 -- tests/templates/test_subroutines/test_trotter.py | 2 +- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/pennylane/templates/subroutines/trotter.py b/pennylane/templates/subroutines/trotter.py index afb3dba2cf2..a7fcf9de801 100644 --- a/pennylane/templates/subroutines/trotter.py +++ b/pennylane/templates/subroutines/trotter.py @@ -234,8 +234,6 @@ def my_circ(c1, c2, time): (tensor(0.00961064, requires_grad=True), tensor(-0.12338274, requires_grad=True), tensor(-5.43401259, requires_grad=True)) """ - grad_method = None - @classmethod def _primitive_bind_call(cls, *args, **kwargs): # accepts no wires, so bypasses the wire processing. diff --git a/tests/templates/test_subroutines/test_trotter.py b/tests/templates/test_subroutines/test_trotter.py index d39bf0f22ba..a5b6ef02ea3 100644 --- a/tests/templates/test_subroutines/test_trotter.py +++ b/tests/templates/test_subroutines/test_trotter.py @@ -495,7 +495,7 @@ def circ_ps(coeffs, time): ) for actual, expected in zip(ps, expected_bp): - assert qml.math.allclose(actual, expected), error_msg + assert qml.math.allclose(actual, expected, atol=1e-6), error_msg # TODO: Remove test when we deprecate ApproxTimeEvolution @pytest.mark.parametrize("n", (1, 2, 5, 10)) From d0740685ce146191fdded1193c63fd3bdf9e22bf Mon Sep 17 00:00:00 2001 From: andrijapau Date: Thu, 2 Jan 2025 16:04:45 -0500 Subject: [PATCH 11/12] fix: Remove grad_method=None addition from exp.py --- pennylane/ops/op_math/exp.py | 1 - 1 file changed, 1 deletion(-) diff --git a/pennylane/ops/op_math/exp.py b/pennylane/ops/op_math/exp.py index bb762bf8ad9..52ac51c1700 100644 --- a/pennylane/ops/op_math/exp.py +++ b/pennylane/ops/op_math/exp.py @@ -162,7 +162,6 @@ class Exp(ScalarSymbolicOp, Operation): """ - grad_method = None control_wires = Wires([]) _name = "Exp" From 0314e41fc379d45bb915dcd7b157fcf8d61fcd39 Mon Sep 17 00:00:00 2001 From: andrijapau Date: Thu, 2 Jan 2025 16:48:17 -0500 Subject: [PATCH 12/12] fix: Update test_trotter.py differentiation test --- tests/templates/test_subroutines/test_trotter.py | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/tests/templates/test_subroutines/test_trotter.py b/tests/templates/test_subroutines/test_trotter.py index a5b6ef02ea3..a0593853f41 100644 --- a/tests/templates/test_subroutines/test_trotter.py +++ b/tests/templates/test_subroutines/test_trotter.py @@ -466,7 +466,6 @@ def test_differentiation(self, hamiltonian): time, n, order = (4.2, 10, 4) dev = qml.device("default.qubit") - time = qml.numpy.array(time) coeffs, ops = hamiltonian.terms() @qml.qnode(dev, diff_method="backprop") @@ -485,8 +484,16 @@ def circ_ps(coeffs, time): qml.TrotterProduct(hamiltonian, time, n, order) return qml.probs() + coeffs = qml.numpy.array(coeffs) + time = qml.numpy.array(time) + expected_bp = qml.jacobian(circ_bp)(coeffs, time) + assert expected_bp[0].shape == (2**hamiltonian.num_wires, len(coeffs)) + assert expected_bp[1].shape == (2**hamiltonian.num_wires,) + ps = qml.jacobian(circ_ps)(coeffs, time) + assert ps[0].shape == (2**hamiltonian.num_wires, len(coeffs)) + assert ps[1].shape == (2**hamiltonian.num_wires,) error_msg = ( "Parameter-shift does not produce the same Jacobian as with backpropagation. "