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.

Sign up for GitHub

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

Jump to bottom

Improve performance of qml.expval using op. arithmetic #3063

Merged
merged 19 commits into from Sep 20, 2022
Merged

Improve performance of qml.expval using op. arithmetic #3063

merged 19 commits into from Sep 20, 2022

Conversation

AlbertMitjans
Copy link
Collaborator

@AlbertMitjans AlbertMitjans commented Sep 15, 2022 •
edited

  • Remove expensive unitary check.

Note: the computation of the expval of qml.Hamiltonian is extremely fast because it has its own implementation using its sparse_matrix representation. Should we do sth similar for the Sum operator?

AlbertMitjans and others added 4 commits September 13, 2022 10:12
@AlbertMitjans
compute math.dot when num_wires > 11 (#3046)
4d169af
@antalszava
flaky test_single_argument_step (#3055)
f09e09a
@albi3ro @rmoyard @antalszava
Exp matrix computation bugfix (#3057)
5835d61 
* Incrementing the version number to `v0.27.0-dev` (#3044)

* Update

* Bump

* re-add dev changelog

* Update doc/development/release_notes.md

* docs section

* flaky test_single_argument_step

Co-authored-by: Antal Szava <antalszava@gmail.com>

* fix exp bug

* rebase onto release candidate

* Update doc/releases/changelog-0.26.0.md

* Update tests/optimize/test_optimize_shot_adapative.py

* add additional test parametrization

Co-authored-by: Romain Moyard <rmoyard@gmail.com>
Co-authored-by: Antal Szava <antalszava@gmail.com>
@AlbertMitjans
remove unitary check
14b9e3e
@github-actions
Copy link
Contributor

Hello. You may have forgotten to update the changelog!
Please edit doc/releases/changelog-dev.md with:

  • A one-to-two sentence description of the change. You may include a small working example for new features.
  • A link back to this PR.
  • Your name (or GitHub username) in the contributors section.

@AlbertMitjans AlbertMitjans changed the base branch from master to v0.26.0-rc0 September 15, 2022 09:08
@AlbertMitjans
Copy link
Collaborator Author

Time taken to compute qml.expval of a Sum of 20 Prod operators containing x random factors in 10 different wires:

  • Before:
    image

  • After:
    image

@codecov
Copy link

codecov bot commented Sep 15, 2022 •
edited

Codecov Report

Merging #3063 (0d51043) into master (d038f20) will decrease coverage by 0.00%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master    #3063      +/-   ##
==========================================
- Coverage   99.67%   99.67%   -0.01%     
==========================================
  Files         273      273              
  Lines       23346    23345       -1     
==========================================
- Hits        23270    23269       -1     
  Misses         76       76
Impacted Files Coverage Δ
pennylane/ops/op_math/composite.py 100.00% <100.00%> (ø)
pennylane/ops/qubit/matrix_ops.py 100.00% <100.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@AlbertMitjans AlbertMitjans added the review-ready 👌 PRs which are ready for review by someone from the core team. label Sep 16, 2022
@AlbertMitjans AlbertMitjans changed the base branch from v0.26.0-rc0 to master September 16, 2022 14:22
@albi3ro albi3ro marked this pull request as ready for review September 16, 2022 19:19
Jaybsoni
Jaybsoni previously requested changes Sep 16, 2022
View reviewed changes
Copy link
Contributor

@Jaybsoni Jaybsoni left a comment

Choose a reason for hiding this comment

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

Nice and quick little improvement 👍🏼!

Just missing a changelog entry and still need to update the class doc-string for QubitUnitrary to mention this new kwarg. It seems that its also missing a section for Returns and Raises where we list the class instance and the value error + user warning that instantiation of this class can raise.

pennylane/ops/qubit/matrix_ops.py Outdated Show resolved Hide resolved
albi3ro
albi3ro requested changes Sep 16, 2022
View reviewed changes
Copy link
Contributor

@albi3ro albi3ro left a comment

Choose a reason for hiding this comment

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

Required changes:

  1. Add unitary_check and Keyword Args: to the docstring for the class.
  2. Fix the warning message.

@albi3ro albi3ro requested a review from mlxd September 16, 2022 19:31
mlxd
mlxd previously requested changes Sep 16, 2022
View reviewed changes
Copy link
Member

@mlxd mlxd left a comment

Choose a reason for hiding this comment

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

Small change request from my side

pennylane/ops/qubit/matrix_ops.py Outdated Show resolved Hide resolved
AlbertMitjans and others added 5 commits September 19, 2022 09:49
@AlbertMitjans @albi3ro
Update pennylane/ops/qubit/matrix_ops.py
efcef78 
Co-authored-by: Christina Lee <christina@xanadu.ai>
@AlbertMitjans @mlxd
Update pennylane/ops/qubit/matrix_ops.py
9bd087f 
Co-authored-by: Lee James O'Riordan <mlxd@users.noreply.github.com>
@AlbertMitjans
address review comments
df78f73
@AlbertMitjans
address review comments
fdad35b
@AlbertMitjans
address review comments
a1f9132
@AlbertMitjans
Copy link
Collaborator Author

Nice and quick little improvement 👍🏼!

Just missing a changelog entry and still need to update the class doc-string for QubitUnitrary to mention this new kwarg. It seems that its also missing a section for Returns and Raises where we list the class instance and the value error + user warning that instantiation of this class can raise.

Added changelog entry and docs for the keyword arguments and the error/warning. I don't think we use a Returns section for class constructors in PL.

@AlbertMitjans
Copy link
Collaborator Author

[sc-26108]

albi3ro
albi3ro approved these changes Sep 19, 2022
View reviewed changes
Copy link
Contributor

@albi3ro albi3ro left a comment

Choose a reason for hiding this comment

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

Looks good to me :)

@AlbertMitjans AlbertMitjans dismissed stale reviews from mlxd and Jaybsoni September 20, 2022 08:34

Applied requested change

@antalszava
Copy link
Contributor

Codecov report is incorrect, overriding.

@antalszava antalszava merged commit da04546 into master Sep 20, 2022
@antalszava antalszava deleted the improve-performance-2 branch September 20, 2022 14:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@albi3ro albi3ro albi3ro approved these changes

@Jaybsoni Jaybsoni Awaiting requested review from Jaybsoni

@mlxd mlxd Awaiting requested review from mlxd

Assignees
No one assigned
Labels
review-ready 👌 PRs which are ready for review by someone from the core team.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@AlbertMitjans @antalszava @mlxd @albi3ro @Jaybsoni