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

Momentum-QNG optimization algo #6240

Merged
merged 40 commits into from
Oct 3, 2024
Merged

Conversation

borbysh
Copy link
Contributor

@borbysh borbysh commented Sep 8, 2024

Before submitting

Please complete the following checklist when submitting a PR:

  • All new features must include a unit test.
    If you've fixed a bug or added code that should be tested, add a test to the
    test directory!

  • All new functions and code must be clearly commented and documented.
    If you do make documentation changes, make sure that the docs build and
    render correctly by running make docs.

  • Ensure that the test suite passes, by running make test.

  • Add a new entry to the doc/releases/changelog-dev.md file, summarizing the
    change, and including a link back to the PR.

  • The PennyLane source code conforms to
    PEP8 standards.
    We check all of our code against Pylint.
    To lint modified files, simply pip install pylint, and then
    run pylint pennylane/path/to/file.py.

When all the above are checked, delete everything above the dashed
line and fill in the pull request template.


Context:

Description of the Change:

Benefits:

Possible Drawbacks:

Related GitHub Issues:

A generalization of the Quantum Natural Gradient (QNG) optimizer by considering a discrete-time Langevin equation with QNG force.
A generalization of the Quantum Natural Gradient (QNG) optimizer by considering a discrete-time Langevin equation
    with QNG force. For details of the theory and derivation of Momentum-QNG, please, see: 
    
        Oleksandr Borysenko, Mykhailo Bratchenko, Ilya Lukin, Mykola Luhanko, Ihor Omelchenko,
        Andrii Sotnikov and Alessandro Lomi.
        "Application of Langevin Dynamics to Advance the Quantum Natural Gradient Optimization Algorithm"
        <https://arxiv.org/abs/2409.01978>

    We are grateful to David Wierichs for his generous help with the multi-argument variant of the MomentumQNGOptimizer class.
    
    In PennyLane, the MomentumQNGOptimizer class is a subclass of the QNGOptimizer class and requires one additional 
    hyperparameter (the momentum coefficient) :math:`0 \leq \rho < 1`, the default value being :math:`\rho=0.9`. For :math:`\rho=0` Momentum-QNG
    reduces to the basic QNG.
A generalization of the Quantum Natural Gradient (QNG) optimizer by considering a discrete-time Langevin equation
    with QNG force. For details of the theory and derivation of Momentum-QNG, please, see: 
    
        Oleksandr Borysenko, Mykhailo Bratchenko, Ilya Lukin, Mykola Luhanko, Ihor Omelchenko,
        Andrii Sotnikov and Alessandro Lomi.
        "Application of Langevin Dynamics to Advance the Quantum Natural Gradient Optimization Algorithm"
        <https://arxiv.org/abs/2409.01978>

    We are grateful to David Wierichs for his generous help with the multi-argument variant of the MomentumQNGOptimizer class.
    
    In PennyLane, the MomentumQNGOptimizer class is a subclass of the QNGOptimizer class and requires one additional 
    hyperparameter (the momentum coefficient) :math:`0 \leq \rho < 1`, the default value being :math:`\rho=0.9`. For :math:`\rho=0` Momentum-QNG
    reduces to the basic QNG.
Copy link

codecov bot commented Sep 8, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 99.70%. Comparing base (1a143d2) to head (4ac363c).
Report is 2 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #6240   +/-   ##
=======================================
  Coverage   99.70%   99.70%           
=======================================
  Files         444      445    +1     
  Lines       42236    42260   +24     
=======================================
+ Hits        42113    42137   +24     
  Misses        123      123           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Contributor

@dwierichs dwierichs left a comment

Choose a reason for hiding this comment

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

Hi @borbysh,
thanks a lot for opening this pull request!
I leave a first batch of small comments on the docstrings/changelog entry :)
Feel free to modify my suggestions further or reject them. Most of them are concerned with bringing the texts a bit closer to the standard PennyLane style.

Let me know how you'd like to proceed regarding tests. :) 🎉

doc/releases/changelog-dev.md Outdated Show resolved Hide resolved
doc/releases/changelog-dev.md Outdated Show resolved Hide resolved
pennylane/optimize/momentum_qng.py Outdated Show resolved Hide resolved
pennylane/optimize/momentum_qng.py Outdated Show resolved Hide resolved
pennylane/optimize/momentum_qng.py Outdated Show resolved Hide resolved
pennylane/optimize/momentum_qng.py Outdated Show resolved Hide resolved
pennylane/optimize/momentum_qng.py Outdated Show resolved Hide resolved
pennylane/optimize/momentum_qng.py Outdated Show resolved Hide resolved
borbysh and others added 6 commits September 9, 2024 22:02
Co-authored-by: David Wierichs <david.wierichs@xanadu.ai>
Co-authored-by: David Wierichs <david.wierichs@xanadu.ai>
Co-authored-by: David Wierichs <david.wierichs@xanadu.ai>
Co-authored-by: David Wierichs <david.wierichs@xanadu.ai>
Co-authored-by: David Wierichs <david.wierichs@xanadu.ai>
Co-authored-by: David Wierichs <david.wierichs@xanadu.ai>
@borbysh
Copy link
Contributor Author

borbysh commented Sep 9, 2024 via email

Copy link
Contributor

@dwierichs dwierichs 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 adding the tests, @borbysh .
I left suggestions where to change the optimizer to the new class.
Then we'll have to adapt the tests to actually expect the right behaviour (namely the one of MomentumQNGOptimizer instead of QNGOptimizer)

The tests are run automatically in the continuous integration, which you can see at the bottom of the pull request overview.
In order to run them locally, install the developer's requirements via

pip install -r requirements-dev.txt

Afterwards, you can run the tests with pytest:

python -m pytest tests/optimizer/test_momentum_qng.py

tests/optimize/test_momentum_qng.py Outdated Show resolved Hide resolved
tests/optimize/test_momentum_qng.py Outdated Show resolved Hide resolved
tests/optimize/test_momentum_qng.py Outdated Show resolved Hide resolved
tests/optimize/test_momentum_qng.py Outdated Show resolved Hide resolved
tests/optimize/test_momentum_qng.py Outdated Show resolved Hide resolved
tests/optimize/test_momentum_qng.py Outdated Show resolved Hide resolved
tests/optimize/test_momentum_qng.py Outdated Show resolved Hide resolved
tests/optimize/test_momentum_qng.py Outdated Show resolved Hide resolved
tests/optimize/test_momentum_qng.py Outdated Show resolved Hide resolved
tests/optimize/test_momentum_qng.py Outdated Show resolved Hide resolved
borbysh and others added 7 commits September 10, 2024 09:23
Co-authored-by: David Wierichs <david.wierichs@xanadu.ai>
Co-authored-by: David Wierichs <david.wierichs@xanadu.ai>
Co-authored-by: David Wierichs <david.wierichs@xanadu.ai>
Co-authored-by: David Wierichs <david.wierichs@xanadu.ai>
Co-authored-by: David Wierichs <david.wierichs@xanadu.ai>
Co-authored-by: David Wierichs <david.wierichs@xanadu.ai>
Copy link
Contributor Author

@borbysh borbysh left a comment

Choose a reason for hiding this comment

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

Dear David!
I made corrections according to your advises.
Regards
Alex

borbysh and others added 7 commits October 3, 2024 11:13
Co-authored-by: Pietropaolo Frisoni <pietropaolo.frisoni@xanadu.ai>
Co-authored-by: Pietropaolo Frisoni <pietropaolo.frisoni@xanadu.ai>
Co-authored-by: Pietropaolo Frisoni <pietropaolo.frisoni@xanadu.ai>
Co-authored-by: Pietropaolo Frisoni <pietropaolo.frisoni@xanadu.ai>
Copy link
Contributor Author

@borbysh borbysh left a comment

Choose a reason for hiding this comment

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

Dear Pietropaolo!
Thank you very much for your review and suggestions!
I accepted all the changes you proposed.
Regards,
Alex

Copy link
Contributor Author

@borbysh borbysh left a comment

Choose a reason for hiding this comment

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

Dear Pietropaolo,
thank you very much for your review and comments!
I made some changes according to your suggestions.
Regards
Alex
P.S. Looks like a duplicate comment, but the more - the better)

@PietropaoloFrisoni
Copy link
Contributor

@borbysh Awesome! I am happy to approve once we have a green CI :)

I think you need to pull from the master branch and resolve the conflicts in the changelog file before the tests start. If you have issues I can take care of it myself (just let me know)

@dwierichs
Copy link
Contributor

I took the liberty to perform the merge :)

Copy link
Contributor Author

@borbysh borbysh left a comment

Choose a reason for hiding this comment

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

I'm a bit confused. I only changed "m_t" to "metric_tensor" in the left-hand side in line 126. I didn't alter the right-hand side and it passed tests 3 weeks ago)

Copy link
Contributor

@PietropaoloFrisoni PietropaoloFrisoni left a comment

Choose a reason for hiding this comment

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

I also took the liberty to fix one line of code with black to pass the test : )

Congrats again @borbysh !🎉

@borbysh
Copy link
Contributor Author

borbysh commented Oct 3, 2024

Thank you very much, David Wierichs and Pietropaolo Frisoni!
I wouldn't manage to ride this beast without your help!

@dwierichs
Copy link
Contributor

@borbysh You wanna push the "squash and merge" button? :)

@borbysh
Copy link
Contributor Author

borbysh commented Oct 3, 2024

With a great pleasure, If you tell me where it is!

@dwierichs
Copy link
Contributor

There should be a large green button above the comment box here.

@borbysh
Copy link
Contributor Author

borbysh commented Oct 3, 2024

I'm in panic. Can't find it

@dwierichs
Copy link
Contributor

Haha, no worries :D
Maybe I'm missing something and you can't perform the action 🤔 I'll ask.

@Alex-Preciado
Copy link
Contributor

No worries, I'm merging this PR now. Thank you so much for your contribution @borbysh 🚀

@Alex-Preciado Alex-Preciado merged commit d9e833a into PennyLaneAI:master Oct 3, 2024
37 checks passed
@PietropaoloFrisoni
Copy link
Contributor

PietropaoloFrisoni commented Oct 3, 2024

@borbysh It's the 'Squash and merge' green tab on the very bottom. I can merge the PR for you if you wish

Edit: Looks like it's no longer necessary : )

Screenshot 2024-10-03 095828

@borbysh
Copy link
Contributor Author

borbysh commented Oct 3, 2024

Thank you very much, guys! I owe you some good drinks!

PietropaoloFrisoni added a commit that referenced this pull request Oct 4, 2024
We fix a couple of typos in the doc discovered while reviewing #6240
astralcai added a commit that referenced this pull request Oct 30, 2024
…QNGOptimizer` update with singular metric tensor (#6471)

**Context:**
The newly added `MomentumQNGOptimizer` in
#6240 does not use the
keyword arguments `approx` and `lam`.

**Description of the Change:**
Pass the unused arguments to `super().__init__`

**Possible Drawbacks:**
No tests are added for this, but these two parameters are inherited from
the base class, and they are never tested in the existing test module
for the base class to begin with.

---------

Co-authored-by: dwierichs <david.wierichs@xanadu.ai>
Co-authored-by: JerryChen97 <chenys13@outlook.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants