Adding PasqalDevice #40

co9olguy · 2020-08-15T19:20:57Z

No description provided.

* adding basic pasqal device * using base simulator and removing qubit worries * adding tests * changes * merge master and fix travis issue * linting * change version * Update pennylane_cirq/pasqal_device.py Co-authored-by: Josh Izaac <josh146@gmail.com> * Update pennylane_cirq/pasqal_device.py Co-authored-by: Josh Izaac <josh146@gmail.com> * Update pennylane_cirq/pasqal_device.py Co-authored-by: Josh Izaac <josh146@gmail.com> Co-authored-by: Josh Izaac <josh146@gmail.com>

* Update requirements.txt * Update requirements.txt * Update requirements.txt * Update pasqal_device.py * Update test_pasqal_device.py Co-authored-by: Josh Izaac <josh146@gmail.com>

co9olguy · 2020-08-15T19:22:09Z

pennylane_cirq/cirq_device.py

+        # For consistency, this plugin uses that same total order
+        self.qubits = sorted(qubits)
+
+        super().__init__(wires, shots, analytic)


I had to move down the superclass initialization here, since it is the thing that creates the wire_map, but the wire_map can't be determined until the cirq qubits have been assigned

@mariaschuld might have some ideas on how to improve Wire/cirq qubit integration (there was an allusion to it in #39 and #37)

I now I understand the following PR cherry picked from this one.

co9olguy · 2020-08-15T19:22:43Z

pennylane_cirq/cirq_device.py

@@ -149,7 +157,10 @@ def reset(self):
        # pylint: disable=missing-function-docstring
        super().reset()

-        self.circuit = cirq.Circuit()
+        if self.cirq_device:
+            self.circuit = cirq.Circuit(self.cirq_device)


Needed in order to use a non-default Cirq device (PasqalVirtualDevice)

co9olguy · 2020-08-15T19:23:06Z

requirements.txt

@@ -1,3 +1,4 @@
 pennylane>=0.9
-cirq>=0.7
+google-api-core[grpc]<=1.14.0
+git+https://github.com/quantumlib/Cirq.git@master


Pasqal functionality now merged into Cirq master, but not released

co9olguy · 2020-08-15T19:23:43Z

tests/test_cirq_device.py

-        assert dev.qubits[3] == cirq.GridQubit(1, 1)
+        assert dev.qubits == qubits
+
+    def test_outer_init_of_qubits_unordered(self):


This was never being tested properly before

co9olguy · 2020-08-15T21:04:40Z

pennylane_cirq/cirq_device.py

@@ -226,3 +244,10 @@ def apply(self, operations, **kwargs):
        # Diagonalize the given observables
        for operation in rotations:
            self._apply_operation(operation)
+
+    def define_wire_map(self, wires):


Trying to get ThreeDQubits to work uncovered a lot of uncovered edge cases at the interaction point between PennyLane's wires and Cirq's qubits

co9olguy · 2020-08-15T21:05:04Z

pennylane_cirq/cirq_device.py

+            ``q1>q2``, then the wire indices 0 and 1 are mapped to q2 and q1, respectively.
+            If the user provides their own wire labels, e.g., ``wires=["alice", "bob"]``, and the
+            qubits are the same as the previous example, then "alice" would map to qubit q2
+            and "bob" would map to qubit q1.


Note: this has a big possibility to confuse users, but I'm not sure there's any better way to deal with it

This is super confusing 🤔

spent most of a day trying to figure this out, the alternative is even more confusing :(

pennylane_cirq/cirq_device.py

co9olguy · 2020-08-15T21:59:58Z

pennylane_cirq/simulator_device.py

        # wires that are not acted upon
-        self.circuit.append(cirq.IdentityGate(len(self.qubits))(*self.qubits))


This line broke the PasqalDevice, since it has restrictions about how close qubits must be in order to have multi-qubit gates. Note that it would break any other cirq device in the future which also had topology restrictions

I'm wondering if applying identity gates to unused qubits is the right approach. It sounds like Cirq is being somewhat smart here, and simply just discarding qubits from the simulation that are not involved in the computation. Pyquil does this same; we should be able to simply take into account these ignored qubits when returning the samples/probabilities.

Yes, but I think this requires a change to how the devices work, doesn't it?

I don't think so, the Forest simulators also act like this, and we take it into account in the PL plugin. We simply keep track of the inactive wires, and 'expand_dim' the probability before returning it.

Yes, then it makes sense to do that here. I think the previous solution (identity on all wires) was a quick hack.

But it might also make sense to provide helper functions for keeping track of such things in the base Device class?

pennylane_cirq/cirq_device.py

josh146 · 2020-08-16T13:26:52Z

pennylane_cirq/cirq_device.py

+            ``q1>q2``, then the wire indices 0 and 1 are mapped to q2 and q1, respectively.
+            If the user provides their own wire labels, e.g., ``wires=["alice", "bob"]``, and the
+            qubits are the same as the previous example, then "alice" would map to qubit q2
+            and "bob" would map to qubit q1.


This is super confusing 🤔

josh146 · 2020-08-16T13:27:54Z

pennylane_cirq/cirq_device.py

+        # For consistency, this plugin uses that same total order
+        self.qubits = sorted(qubits)
+
+        super().__init__(wires, shots, analytic)


@mariaschuld might have some ideas on how to improve Wire/cirq qubit integration (there was an allusion to it in #39 and #37)

josh146 · 2020-08-16T13:31:41Z

pennylane_cirq/simulator_device.py

        # wires that are not acted upon
-        self.circuit.append(cirq.IdentityGate(len(self.qubits))(*self.qubits))


I'm wondering if applying identity gates to unused qubits is the right approach. It sounds like Cirq is being somewhat smart here, and simply just discarding qubits from the simulation that are not involved in the computation. Pyquil does this same; we should be able to simply take into account these ignored qubits when returning the samples/probabilities.

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

co9olguy · 2020-10-08T15:30:18Z

@josh146 Not sure why CodeFactor is complaining about Iterable, it imports fine when I run locally 🤔

josh146 · 2020-10-08T15:44:20Z

@josh146 Not sure why CodeFactor is complaining about Iterable, it imports fine when I run locally 🤔

This is a known bug that is safe to ignore for now. The issue stems from the fact that collections.Iterable is deprecated, and the new home is collections.abc.Iterable -- this will be breaking in Python 3.9. Until then, the way Python makes this work from both import locations confuses pylint.

co9olguy · 2020-10-08T15:48:43Z

docker build for black check erroring as well, but I think it's an external issue 🤔

josh146

Looks good @co9olguy! A couple of high level comments before merging:

the permissions changes on the doc/xanadu_theme folder should be reverted (git checkout master doc/xanadu_theme).
The Pasqal device hasn't been added to the docs --- a card should be added on the doc/index.rst page, and a new page added in the doc/devices/ directory. The information in the class docstring is not very user accessible (they would need to dig through the API to find it).
We should update the setup.py requirements.

pennylane_cirq/cirq_device.py

requirements.txt

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

josh146 · 2020-10-08T16:06:16Z

docker build for black check erroring as well, but I think it's an external issue 🤔

This is now fixed, it's failing still though because Black is complaining on an unformatted file :)

co9olguy · 2020-10-08T16:06:45Z

docker build for black check erroring as well, but I think it's an external issue thinking

This is now fixed, it's failing still though because Black is complaining on an unformatted file :)

That is easy enough to deal with :)

josh146

💯

doc/devices/pasqal.rst

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

codecov · 2020-10-08T17:02:22Z

Codecov Report

❗ No coverage uploaded for pull request base (master@e985c4b). Click here to learn what that means.
The diff coverage is 100.00%.

@@            Coverage Diff            @@
##             master      #40   +/-   ##
=========================================
  Coverage          ?   99.60%           
=========================================
  Files             ?        7           
  Lines             ?      253           
  Branches          ?        0           
=========================================
  Hits              ?      252           
  Misses            ?        1           
  Partials          ?        0

Impacted Files	Coverage Δ
pennylane_cirq/cirq_device.py	`98.75% <ø> (ø)`
pennylane_cirq/cirq_operation.py	`100.00% <ø> (ø)`
pennylane_cirq/__init__.py	`100.00% <100.00%> (ø)`
pennylane_cirq/pasqal_device.py	`100.00% <100.00%> (ø)`
pennylane_cirq/_version.py	`100.00% <0.00%> (ø)`
pennylane_cirq/ops.py	`100.00% <0.00%> (ø)`
pennylane_cirq/simulator_device.py	`100.00% <0.00%> (ø)`
... and 1 more

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 e985c4b...c3973ff. Read the comment docs.

co9olguy and others added 11 commits July 8, 2020 13:11

Update branch (#33)

92872e2

* Update requirements.txt * Update requirements.txt * Update requirements.txt * Update pasqal_device.py * Update test_pasqal_device.py Co-authored-by: Josh Izaac <josh146@gmail.com>

change coords

3270b3a

Merge branch 'master' into pasqal-holding

7962f6b

fix

a083d43

proper attaching of pasqal device

bf27941

debugging

0638871

cleanup and explanation

bc4385f

updates for failing tests

db54ef3

updating requirements to master branch of cirq

5e895f5

ordering test upgrades

b6aae21

co9olguy commented Aug 15, 2020

View reviewed changes

fixing more edge cases

493322c

co9olguy commented Aug 15, 2020

View reviewed changes

pennylane_cirq/cirq_device.py Outdated Show resolved Hide resolved

co9olguy added 3 commits August 15, 2020 17:05

Update pennylane_cirq/cirq_device.py

3f91da1

removing print

83d52f0

make identity gates local

e3703c0

co9olguy commented Aug 15, 2020

View reviewed changes

josh146 reviewed Aug 16, 2020

View reviewed changes

josh146 mentioned this pull request Aug 18, 2020

Fixes for device wire handling #42

Merged

co9olguy and others added 3 commits October 8, 2020 11:08

Merge branch 'master' into pasqal-holding

defe579

Update pennylane_cirq/cirq_device.py

779ae95

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

imports

df0447f

black

b3692de

Update format.yml

63d5199

josh146 reviewed Oct 8, 2020

View reviewed changes

pennylane_cirq/cirq_device.py Outdated Show resolved Hide resolved

requirements.txt Show resolved Hide resolved

Update pennylane_cirq/cirq_device.py

f6b5f13

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

co9olguy added 5 commits October 8, 2020 12:11

blacker

37a7afa

fixes

104dada

docs

093b99f

more dox

aad2488

the

a1d5432

josh146 approved these changes Oct 8, 2020

View reviewed changes

doc/devices/pasqal.rst Outdated Show resolved Hide resolved

josh146 and others added 4 commits October 9, 2020 00:49

Update tests.yml

8a4547c

Update doc/devices/pasqal.rst

22fd570

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

Update tests.yml

33c9a06

Update tests.yml

d28563c

Update tests.yml

c3973ff

co9olguy merged commit f3dc1ff into master Oct 8, 2020

co9olguy deleted the pasqal-holding branch October 8, 2020 17:09

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adding PasqalDevice #40

Adding PasqalDevice #40

co9olguy commented Aug 15, 2020

co9olguy Aug 15, 2020

josh146 Aug 16, 2020

mariaschuld Aug 19, 2020

co9olguy Aug 15, 2020

co9olguy Aug 15, 2020

co9olguy Aug 15, 2020

co9olguy Aug 15, 2020

co9olguy Aug 15, 2020

josh146 Aug 16, 2020

co9olguy Aug 16, 2020

co9olguy Aug 15, 2020

josh146 Aug 16, 2020

co9olguy Aug 16, 2020

josh146 Aug 16, 2020

co9olguy Aug 16, 2020

josh146 Aug 16, 2020

josh146 Aug 16, 2020

josh146 Aug 16, 2020

co9olguy commented Oct 8, 2020

josh146 commented Oct 8, 2020

co9olguy commented Oct 8, 2020

josh146 left a comment

josh146 commented Oct 8, 2020

co9olguy commented Oct 8, 2020

josh146 left a comment

codecov bot commented Oct 8, 2020 •

edited

Loading

		# wires that are not acted upon
		self.circuit.append(cirq.IdentityGate(len(self.qubits))(*self.qubits))

Adding PasqalDevice #40

Adding PasqalDevice #40

Conversation

co9olguy commented Aug 15, 2020

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

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

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

co9olguy commented Oct 8, 2020

josh146 commented Oct 8, 2020

co9olguy commented Oct 8, 2020

josh146 left a comment

Choose a reason for hiding this comment

josh146 commented Oct 8, 2020

co9olguy commented Oct 8, 2020

josh146 left a comment

Choose a reason for hiding this comment

codecov bot commented Oct 8, 2020 • edited Loading

Codecov Report

codecov bot commented Oct 8, 2020 •

edited

Loading