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

added feature to decompose hamiltonians to Pauli matrices #535

Merged

Conversation

rahul1704
Copy link
Contributor

@rahul1704 rahul1704 commented Mar 7, 2020

Context: Currently, if a user has a numerical matrix, they must either use qml.Hermitian in a single QNode (causing additional classical processing/requiring full subsystem rotations), or decompose it manually into Pauli operators to be used with qml.map().

Description of the Change: Adds a function that allows users to easily decompose their Hamiltonian into Pauli operators.

Benefits: Allows numeric Hamiltonians to be easily used with qml.map().

Possible Drawbacks: n/a

Related GitHub Issues:

Closes #527

Provides another solution to #553

@rahul1704 rahul1704 closed this Mar 7, 2020
@co9olguy
Copy link
Member

co9olguy commented Mar 7, 2020

Thanks for the pull request @rahul1704!
How come you have closed it after submission?

@rahul1704
Copy link
Contributor Author

Hey @co9olguy! I saw it failed a few tests, so I was unsure if I should keep it open and fix it or close it, for now, fix the issues and then create a new one. Let me know which is preferable

@co9olguy
Copy link
Member

co9olguy commented Mar 8, 2020

No problem to keep open. Work in progress PRs often fail tests early on :)

@co9olguy co9olguy reopened this Mar 8, 2020
@codecov
Copy link

codecov bot commented Mar 8, 2020

Codecov Report

❗ No coverage uploaded for pull request base (decompose-hamiltonian@30dd7d4). Click here to learn what that means.
The diff coverage is n/a.

@@                   Coverage Diff                    @@
##             decompose-hamiltonian     #535   +/-   ##
========================================================
  Coverage                         ?   98.93%           
========================================================
  Files                            ?       83           
  Lines                            ?     5158           
  Branches                         ?        0           
========================================================
  Hits                             ?     5103           
  Misses                           ?       55           
  Partials                         ?        0           

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 30dd7d4...1e03f17. Read the comment docs.

@josh146
Copy link
Member

josh146 commented Mar 9, 2020

Hi @rahul1704, it appears that some of the checks are still failing:

  • For the Formatting Check, it's as simple as blacking your code. Simply install the black Python formatter (pip install black) and then run it on all files you've modified, making sure to specify a line width of 100: black -l 100 pennylane/

  • For the test suite, the error causing the test failure is NameError: name 'operator' is not defined. Is operator being defined in the test?

@josh146 josh146 added WIP 🚧 Work-in-progress enhancement ✨ New feature or request labels Mar 9, 2020
@rahul1704
Copy link
Contributor Author

Hi @josh146 @co9olguy,
Thanks for the help. I have reformatted the files and ran the tests again and everything seems okay now.

@co9olguy co9olguy requested a review from josh146 March 10, 2020 20:18
Copy link
Member

@josh146 josh146 left a comment

Choose a reason for hiding this comment

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

Thanks for this @rahul1704! I've left some comments and suggestions. Don't forget to also update the .github/CHANGELOG.md file with the change, a link back to this PR, and to add your name to the contributors list!

pennylane/utils.py Outdated Show resolved Hide resolved
pennylane/utils.py Outdated Show resolved Hide resolved
pennylane/utils.py Outdated Show resolved Hide resolved
pennylane/utils.py Show resolved Hide resolved
Comment on lines +45 to +46
if len(H) - 2 ** N != 0:
raise ValueError("Hamiltonian should be in the form (n^2 x n^2), for any n>=1")
Copy link
Member

Choose a reason for hiding this comment

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

Nice! Very useful to have this dimension validation.

pennylane/utils.py Outdated Show resolved Hide resolved
)
for term in zip(coeff, matrices):
result = result + term[0] * term[1]
assert (H == result).all
Copy link
Member

Choose a reason for hiding this comment

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

This is a good first attempt for a functional test, however, it isn't sufficient --- additional unit and functional tests should be added. These include:

  • Working out, by hand, the decompositions of several different Hamiltonians. Then, run the test (using @pytest.mark.parametrize()) on this test data, verifying that the expected result is returned.

  • Checking that coeffs is a list of floats,

  • Checking that len(coeffs) == len(obs_list), etc.

Choose a reason for hiding this comment

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

Can you give me an example of a Hamiltonian that must be decomposed into its component parts? Or I can actually find that myself. But would any Hermitian matrix suffice?

Copy link
Member

Choose a reason for hiding this comment

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

Hi @yishairasowsky, yes, any Hermitian matrix will suffice!

Choose a reason for hiding this comment

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

i got an error when i tried to push...
remote: Permission to XanaduAI/pennylane.git denied to yishairasowsky.
fatal: unable to access 'https://github.com/XanaduAI/pennylane.git/': The requested URL returned error: 403

Copy link
Member

Choose a reason for hiding this comment

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

Hi @yishairasowsky, that's no problem. For external contributors, it's not possible to directly push to the repo. You'll can push to a fork of PennyLane which is on your own GitHub account, then make a pull request from there to github.com/XanaduAI's decompose-hamiltonian branch


@pytest.mark.parametrize("H", test_Hamiltonians)
def test_decomp(self, H):
"""Tests that decompose_hamiltonian successfully decomposes Hamiltonians into Pauli matrices"""
Copy link
Member

Choose a reason for hiding this comment

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

This docstring can be made more specific --- what are we explicitly comparing? How do we determine the decomposition was successful?

@@ -63,6 +65,33 @@
U_toffoli[6:8, 6:8] = np.array([[0, 1], [1, 0]])


test_Hamiltonians = [np.array([[2.5, -0.5], [-0.5, 2.5]]), np.array(np.diag([0, 0, 0, 1]))]
Copy link
Member

Choose a reason for hiding this comment

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

It would be good to also consider multi-qubit cases, as well as Hermitian matrices with complex off-diagonal entries.

Comment on lines +79 to +91
result = np.zeros((2 ** N, 2 ** N))
matrices = []
for term in combs:
base = [qml.Identity] * N
if term.num_wires == N:
matrices.append(term.matrix)
else:
base[term.wires[0]] = term.__class__
matrices.append(
functools.reduce(operator.matmul, [t(i) for i, t in enumerate(base)]).matrix
)
for term in zip(coeff, matrices):
result = result + term[0] * term[1]
Copy link
Member

Choose a reason for hiding this comment

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

This is quite neat; essentially you are decomposing a Hamiltonian using the function, and then evaluating the linear combination by making use of the observable.matrix property. Nice! However, it could be done in a slightly cleaner manner, without re-using the logic from the function to be tested, like so:

mats = []

for c, o in zip(coeff, obs_list):
    base = [np.identity(2)] * N

    if isinstance(o, qml.operation.Tensor):
        for obs in o.obs:
            base[obs.wires[0]] = obs.matrix
    else:
        base[o.wires[0]] = o.matrix

    mats.append(c*functools.reduce(np.kron, base))

result = np.linalg.multi_dot(mats)

Note: I have note checked the above for correctness, it could need adjusting

@josh146
Copy link
Member

josh146 commented Mar 25, 2020

@rahul1704, how is everything going on this PR? Is it something you would like to continue working on?

@co9olguy
Copy link
Member

@yishairasowsky This PR got part way, but appears to have stalled out. If you're interested (and depending on your level of comfort), it might make a good candidate to help carry across the finish line

@yishairasowsky
Copy link

yishairasowsky commented Apr 30, 2020 via email

@yishairasowsky
Copy link

yishairasowsky commented Apr 30, 2020

i am trying to open the version that @rahul1704 made with the incoming changes for hamiltionian. but i can't find that branch. when i type https://github.com/yishairasowsky/pennylane/tree/hamiltonianDecomp into my address bar, i et a 404 not found.

@co9olguy
Copy link
Member

co9olguy commented Apr 30, 2020

i will look into it. let me check each reviewer comment, and if it is not addressed already, i will try to do so.

@yishairasowsky Amazing, thanks for helping out! 😃

@josh146
Copy link
Member

josh146 commented May 1, 2020

Hi @yishairasowsky, thanks for helping out on this!

Does the following work for you?

git clone https://github.com/rahul1704/pennylane.git
cd pennylane
git checkout hamiltonianDecomp

@yishairasowsky
Copy link

yishairasowsky commented May 1, 2020 via email

@yishairasowsky
Copy link

yishairasowsky commented May 6, 2020

@josh146 thanks for your help.
i am in the rahul branch, and am making the appropriate changes. but when i push, i get an error:
image

pennylane/utils.py Outdated Show resolved Hide resolved
@josh146 josh146 changed the base branch from master to decompose-hamiltonian May 6, 2020 15:34
pennylane/utils.py Outdated Show resolved Hide resolved
@josh146 josh146 merged commit b93fe67 into PennyLaneAI:decompose-hamiltonian May 6, 2020
@josh146
Copy link
Member

josh146 commented May 6, 2020

Hi @yishairasowsky, I've merged this WIP PR into the branch decompose-hamiltonian. You can clone it and begin working on it via the following:

  1. Make sure your fork of PennyLane is up to date

  2. Checkout the decompose-hamiltonian branch:

    git checkout decompose-hamiltonian
  3. When ready, make a pull request here :)

@yishairasowsky
Copy link

Hi @josh146! Thanks a lot. Just checking: To make sure I am up to date, I just made a pull. That is sufficient, right?
Now I will proceed to checkout decompose-hamiltonian.
Thanks!

@co9olguy
Copy link
Member

co9olguy commented May 6, 2020

Yes, if you are successfully able to pull from the decompose-hamiltonian branch Josh just created, you should be good to go!

example (assuming you are on a local clone of pennylane):

git fetch
git checkout decompose-hamiltonian

@yishairasowsky
Copy link

yishairasowsky commented May 6, 2020 via email

@yishairasowsky
Copy link

sry! when i clone my fork, then try git checkout decompose-hamiltonian, i get an error: error: pathspec 'decompose-hamiltonian' did not match any file(s) known to git

@yishairasowsky
Copy link

OK, i see now it is OK.
PS C:\Users\Yishai\Documents\GitHub\pennylane> git checkout decompose-hamiltonian
Switched to a new branch 'decompose-hamiltonian'
Branch 'decompose-hamiltonian' set up to track remote branch 'decompose-hamiltonian' from 'upstream'.
PS C:\Users\Yishai\Documents\GitHub\pennylane>

@co9olguy
Copy link
Member

co9olguy commented May 7, 2020

You got it :)

@yishairasowsky
Copy link

i wrote git remote add pennylane https://github.com/yishairasowsky/pennylane.git in order that my pushes should go to my fork. but still when i type git push, i get the error
PS C:\Users\Yishai\Documents\GitHub\pennylane> git remote add pennylane-yishai https://github.com/yishairasowsky/pennylane.git
PS C:\Users\Yishai\Documents\GitHub\pennylane> git push
remote: Permission to XanaduAI/pennylane.git denied to yishairasowsky.
fatal: unable to access 'https://github.com/XanaduAI/pennylane.git/': The requested URL returned error: 403

@co9olguy
Copy link
Member

co9olguy commented May 7, 2020

Looks like push is still resolving the default upstream branch as the official PennyLane.
Try git push --set-upstream pennylane-yishai branchname where branchname is the name of the branch you have set up on your own github account

@yishairasowsky
Copy link

thanks! hmm, i get
PS C:\Users\Yishai\Documents\GitHub\pennylane> git push --set-upstream pennylane-yishai pennylane
error: src refspec pennylane does not match any
error: failed to push some refs to 'https://github.com/yishairasowsky/pennylane.git'

@yishairasowsky
Copy link

another thing i tried and failed
PS C:\Users\Yishai\Documents\GitHub\pennylane> git remote set-url origin https://github.com/yishairasowsky/pennylane
PS C:\Users\Yishai\Documents\GitHub\pennylane> git push
remote: Permission to XanaduAI/pennylane.git denied to yishairasowsky.
fatal: unable to access 'https://github.com/XanaduAI/pennylane.git/': The requested URL returned error: 403
PS C:\Users\Yishai\Documents\GitHub\pennylane>

@co9olguy
Copy link
Member

co9olguy commented May 7, 2020

The tricky thing about git push is it falls back to a default you have previously set, which is not always origin master.

If you do git push origin master it should work (assuming that you have focked pennylane to your own repo and there are no merge conflicts)

@co9olguy
Copy link
Member

co9olguy commented May 7, 2020

Just for clarity, your above attempts have likely created the aliases origin and pennylane-yishai for the remote server https://github.com/yishairasowsky/pennylane. But your default target for push still seems to be the official pennylane

Check out https://stackoverflow.com/a/18801178/4680447 for some details, but the -u option can be used to set the default, e.g. git push -u origin master

@yishairasowsky
Copy link

yishairasowsky commented May 7, 2020

OK, thanks! hmm, i am getting

PS C:\Users\Yishai\Documents\GitHub\pennylane> git push -u origin master
To https://github.com/yishairasowsky/pennylane.git
 ! [rejected]        master -> master (non-fast-forward)
error: failed to push some refs to 'https://github.com/yishairasowsky/pennylane.git'
hint: Updates were rejected because a pushed branch tip is behind its remote
hint: counterpart. Check out this branch and integrate the remote changes
hint: (e.g. 'git pull ...') before pushing again.
hint: See the 'Note about fast-forwards' in 'git push --help' for details.
PS C:\Users\Yishai\Documents\GitHub\pennylane> git push -u origin pennylane-yishai
error: src refspec pennylane-yishai does not match any
error: failed to push some refs to 'https://github.com/yishairasowsky/pennylane.git'
PS C:\Users\Yishai\Documents\GitHub\pennylane> git push -u pennylane pennylane-yishai
error: src refspec pennylane-yishai does not match any
error: failed to push some refs to 'https://github.com/yishairasowsky/pennylane.git'

did i goof something?

@yishairasowsky
Copy link

yishairasowsky commented May 8, 2020 via email

josh146 added a commit that referenced this pull request Jun 22, 2020
* added feature to decompose hamiltonians to Pauli matrices (#535)

* addded hamiltonians decomp feature

* reformatted utils.py and tests/test_utils.py

* Apply suggestions from code review

* Apply suggestions from code review

* Update utils.py

* Update qml_utils.rst

* Apply suggestions from code review

Co-authored-by: Rahul Shekhawat <rahul@Rahuls-MacBook-Pro.local>
Co-authored-by: Rahul Shekhawat <rahul@Rahuls-MBP.mshome.net>
Co-authored-by: Nathan Killoran <co9olguy@users.noreply.github.com>
Co-authored-by: Josh Izaac <josh146@gmail.com>

* Updated code for decomposition; added tests

* Fixed formatting

* Update pennylane/utils.py

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

* Update pennylane/utils.py

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

* Update pennylane/utils.py

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

* Update tests/test_utils.py

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

* Updates following PR

* Moved hide_identity usage

* Updated docstring

* Update pennylane/utils.py

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

* Added 2 tests

* Update pennylane/utils.py

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

* Updated test

* Updated and added tests

* Update pennylane/utils.py

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

* Update pennylane/utils.py

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

* Update pennylane/utils.py

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

* Update tests/test_utils.py

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

* Updated test string

* Updated exception message

* Refactored shape check

* Updated tests descriptions

* Reformatted

* Updated CHANGELOG

* Update tests/test_utils.py

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

* Update tests/test_utils.py

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

* Update tests/test_utils.py

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

Co-authored-by: Rahul Shekhawat <rahul170492@gmail.com>
Co-authored-by: Rahul Shekhawat <rahul@Rahuls-MacBook-Pro.local>
Co-authored-by: Rahul Shekhawat <rahul@Rahuls-MBP.mshome.net>
Co-authored-by: Nathan Killoran <co9olguy@users.noreply.github.com>
Co-authored-by: Josh Izaac <josh146@gmail.com>
Co-authored-by: Tom Bromley <49409390+trbromley@users.noreply.github.com>
Co-authored-by: trbromley <brotho02@gmail.com>
@yishairasowsky
Copy link

yishairasowsky commented Nov 10, 2020

hi @co9olguy
Is there any chance you would be willing to hiring someone who is excited to contribute to your project? You may remember that my background is in quantum physics and machine learning. My thesis is here: http://citeseerx.ist.psu.edu/viewdoc/download?doi=10.1.1.472.6264&rep=rep1&type=pdf
yoel_manager_recommendation_2020_july_19.pdf
my manager recommendation and resume is attached to this as a file
rasowsky_resume_2020_09_01.pdf
thank you for your consideration!
best wishes,
yishai

@co9olguy
Copy link
Member

co9olguy commented Nov 11, 2020

Hi @yishairasowsky, if you're interested in employment opportunities at Xanadu, you should check out the opportunities on the careers page. GitHub is better used for discussing contributions to the open-source library.

Good luck with your job search :)

@yishairasowsky
Copy link

yishairasowsky commented Nov 11, 2020 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement ✨ New feature or request WIP 🚧 Work-in-progress
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add function decomposing Hamiltonian to Pauli observables
4 participants