k-local Ising wrapper for QAOA by markf94 · Pull Request #141 · rigetti/grove

markf94 · 2018-03-30T05:05:12Z

This PR implements the following changes:

implements support for k-local Ising instances
fixes several bugs that occured with Ising instances that didn't have bias (h) terms for each qubit
introduces proper Read the Docs for the Ising QAOA wrapper
introduces a new parameter driver_operators for ising_qaoa() in order to allow for manual definition of the QAOA mixer hamiltonian
renames ising() to ising_qaoa()

Ising instances can have many variables but almost no bias terms so a dictionary is naturally a better choice in order to account for sparse bias vectors

Ising instances can make use of arbitrary qubits especially when we have to map to a QPU hardware graph. Makes much more sense to extract the qubit indices and compute n_nodes like this.

allows for user-defined mixer/driver hamiltonian in order to enforce hard constraints tests & docs also updated

added extensive documentation for the Ising QAOA wrapper added images added ising_qaoa to index.rst

ampolloreno

@markf94, thank for the PR - and sorry for the (extreme) delay in reviewing. I'd really like to get this merged in.

I've commented in a couple places in the docs for starters, one particularly troubling thing to me is that the examples in the docs seem to return different results than those listed when I run them.

If you don't mind, I'll try to push changes as I go (time permitting), so we can get this merged in.

Thanks again, and looking forward to getting this merged int!

ampolloreno · 2018-05-25T19:30:14Z

docs/ising_qaoa.rst

+Here you can find documentation for the different functions of Ising QAOA.
+
+grove.ising.ising_qaoa.ising_qaoa
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~


Sphinx is complaining here,

/home/ampolloreno/repos/grove/docs/ising_qaoa.rst:256: WARNING: Title underline too short.

Extend the tildes, please.

ampolloreno · 2018-05-25T19:37:37Z

docs/ising_qaoa.rst

+.. image:: ising_qaoa/triangle_desired.png
+   :align: center
+   :scale: 75%
+Let's again define that we colour vertex \\( i \\) black if \\( \\sigma_{i} = -1 \\) and white if \\( \\sigma_{i} = +1 \\).


Sphinx complained here too.
I think it wants a newline after the image block.

/home/ampolloreno/repos/grove/docs/ising_qaoa.rst:218: WARNING: Explicit markup ends without a blank line; unexpected unindent.

ampolloreno · 2018-06-18T19:14:14Z

docs/ising_qaoa.rst

+            stats[tuple(solution_string)] = 1
+    print(f'Solution statistics: {stats}')
+
+You should see that the algorithm returns the aforementioned solution with ~99% probability (unless the classical minimizer got stuck in a local minima).


I don't see this when I run the routine - has something changed that breaks this example since you made this PR?

@ampolloreno the problem was the declaration of driver_operators as an empty list before checking if it is None which resulted in no driver Hamiltonian. This has been resolved in commit fc381d4.

Awesome, I'll check it out!

ampolloreno · 2018-06-18T19:18:33Z

docs/ising_qaoa.rst

+            stats[tuple(solution_string)] = 1
+    print(f'Solution statistics: {stats}')
+
+You should get the two possible solution strings \\( [-1, 1, 1, -1] \\) and \\( [1, -1, -1, 1] \\)


I don't get this either.

@ampolloreno This has also been resolved in commit fc381d4.

ampolloreno

@markf94 Some more comments.

ampolloreno · 2018-06-18T19:47:56Z

grove/ising/ising_qaoa.py

-    :param h: External magnectic term of the Ising problem. List.
-    :param J: Interaction term of the Ising problem. Dictionary.
-    :param sol: Ising solution. List.
+    :param h: (dict) External magnetic term of the Ising problem.


Typically the type goes inside of the parameter specifier, e.g. :param dict h:, I don't know if sphinx renders the type the way it's written here.

ampolloreno · 2018-06-18T19:49:29Z

grove/ising/ising_qaoa.py

-    :param sol: Ising solution. List.
+    :param h: (dict) External magnetic term of the Ising problem.
+    :param J: (dict) Interaction terms of the Ising problem (may be k-local).
+    :param sol: (list) Ising solution.


We should say as returned from the QPU/QVM, given that the code depends on that ordering, and reverses it.

ampolloreno · 2018-06-18T19:58:02Z

grove/ising/ising_qaoa.py

-            raise TypeError("""Interaction term must connect two different variables""")
+        paired_indices =  [(a, b) for a, b in zip(elm, elm)]
+        if len(paired_indices) != len(set(paired_indices)):
+            raise TypeError("Interaction term must connect different variables. "


To catch this can't we just check len(elm) == len(set(elm))?

ampolloreno · 2018-06-18T20:54:58Z

grove/ising/ising_qaoa.py

        else:
-            ener_ising += J[elm] * int(sol[elm[0]]) * int(sol[elm[1]])
-    for i in range(len(h)):
+            multipliers = int(sol[elm[0]]) * int(sol[elm[1]])


I don't know what's special about the first two elements, can we instead do:

multipliers = 1 for idx in elm: multipliers *= sol[idx]

?

ampolloreno · 2018-06-18T22:29:30Z

grove/ising/ising_qaoa.py

+    :param J: (dict) Interaction terms of the Ising problem (may be k-local).
    :param num_steps: (Optional.Default=2 * len(h)) Trotterization order for the
                  QAOA algorithm.
+    :param driver_operators: (Optional. Default: X on all qubits.) The mixer/driver


:param list driver_operators:?

ampolloreno · 2018-06-18T22:32:08Z

grove/ising/ising_qaoa.py

    :param samples: (Optional. Default=None) VQE option. Number of samples
                    (circuit preparation and measurement) to use in operator
-                    averaging.
+                    averaging. Required when using QPU backend.


Is this only required for the QPU? Don't the QVM and QPU both have default num_trials?

ampolloreno · 2018-06-18T22:33:59Z

grove/ising/ising_qaoa.py

        num_steps = 2 * len(h)

-    n_nodes = len(h)
+    qubit_indices = set([ index for tuple_ in list(J.keys()) for index in tuple_]


No space before index.

ampolloreno · 2018-06-18T22:42:38Z

grove/ising/ising_qaoa.py

+        driver_operators = []
+        # default to X mixer
+        for i in qubit_indices:
+            driver_operators.append(PauliSum([PauliTerm("X", i, -1.0)]))


Why is there a -1?

- fixed doc strings - fixed spacing according to PEP8

- declaring driver_operators as empty list led to no driver Hamiltonian being used

markf94 · 2018-07-23T15:11:27Z

@ampolloreno Semaphore build fails due to unrelated issues with Tomography.

ampolloreno · 2018-07-23T16:02:08Z

@markf94 Awesome! I'll get to looking at this again soon!

markf94 and others added 20 commits March 30, 2018 00:35

added proper doc string to ising_trans()

2e9b17b

changed h argument from list to dict

3651e56

Ising instances can have many variables but almost no bias terms so a dictionary is naturally a better choice in order to account for sparse bias vectors

extracting qubit indices from h and J

25253f8

Ising instances can make use of arbitrary qubits especially when we have to map to a QPU hardware graph. Makes much more sense to extract the qubit indices and compute n_nodes like this.

renamed ising() to ising_qaoa() to make clear that it is a QAOA wrapper

38a1854

cleaned up doc string for better read the docs later on

3dec036

supporting k-local interaction terms in J

e978b70

energy_value() now supports k-local interaction terms

bfc6ea7

fixed existing tests for ising_qaoa()

229c7a6

more detailed tests for ising_qaoa()

b5494a8

new tests added for ising_trans() and energy_value()

7a6f819

driver_operators added to arguments

33227ee

allows for user-defined mixer/driver hamiltonian in order to enforce hard constraints tests & docs also updated

read the docs for Ising QAOA

c10dd1e

added extensive documentation for the Ising QAOA wrapper added images added ising_qaoa to index.rst

changed IsingQAOA notebook to new syntax

a73d510

cleaned up doc string for better read the docs later on

d077b07

fixed existing tests for ising_qaoa()

d5ca6e7

new tests added for ising_trans() and energy_value()

d8dbfe3

read the docs for Ising QAOA

3fcf6ab

added extensive documentation for the Ising QAOA wrapper added images added ising_qaoa to index.rst

ising_qaoa: Make Python2 compatible

f447273

tests: Do not use start imports

17a3a3a

docs: Fix incorrect import

8e424a5

markf94 mentioned this pull request Mar 30, 2018

New 'embedding' argument for QAOA #143

Open

style: Minor PEP8 fixes

af3cd11

tbabej force-pushed the ising_qaoa_clean branch from 3a82ff7 to af3cd11 Compare March 30, 2018 06:16

tbabej and others added 2 commits April 9, 2018 16:24

ising_qaoa: Fix energy value computation

848cfc9

fixed tests to accommodate new energy computation

c923a5f

ampolloreno suggested changes Jun 18, 2018

View reviewed changes

markf94 added 3 commits July 22, 2018 18:58

sphinx bug fixes implemented

46d6a05

implemented some requested changes

97c073b

- fixed doc strings - fixed spacing according to PEP8

fixed driver_operators bug

fc381d4

- declaring driver_operators as empty list led to no driver Hamiltonian being used

markf94 added 2 commits July 23, 2018 10:30

simplified test for duplicate elements in J.keys()

11b012a

initializing multipliers + removed -1.0 from driver_ops

a652814

Conversation

markf94 commented Mar 30, 2018

Uh oh!

ampolloreno left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ampolloreno Jun 18, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

markf94 Jul 23, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ampolloreno left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

markf94 commented Jul 23, 2018

Uh oh!

ampolloreno commented Jul 23, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

ampolloreno left a comment •

edited

Loading

ampolloreno Jun 18, 2018 •

edited

Loading

markf94 Jul 23, 2018 •

edited

Loading