Skip to content
This repository has been archived by the owner on Dec 7, 2021. It is now read-only.

Optimizers for noisy quantum devices (scikit-quant) #1240

Merged
merged 33 commits into from
Sep 28, 2020

Conversation

Brogis1
Copy link
Contributor

@Brogis1 Brogis1 commented Sep 9, 2020

Summary

Interface to python implementation of SNOBFIT, IMFIL, BOBYQA classical optimizers from the scikit-quant package (https://github.com/scikit-quant/scikit-quant).
Also see: https://qat4chem.lbl.gov/software

Details and comments

  • pip install scikit-quant to install the optimizers (sckit-quant has been added to the requirements-dev.txt).

  • The implementation in Qiskit follows exact template of already implemented scipy optimizers.

  • A test is included for the 3 optimizers.

  • The keywords were added to pylintdic.

@Brogis1 Brogis1 marked this pull request as draft September 9, 2020 15:26
@Brogis1 Brogis1 marked this pull request as ready for review September 9, 2020 15:28
@adekusar-drl adekusar-drl self-requested a review September 9, 2020 15:29
pbark
pbark previously approved these changes Sep 10, 2020
Copy link
Member

@woodsp-ibm woodsp-ibm left a comment

Choose a reason for hiding this comment

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

Is this intended to be an optional install that would not be installed normally when the user installs Aqua? Is there some reason for it to be optional?

If the intent is for it to be optional then the imports and unit tests need to handle the case where its not installed as is done in the various other optionally installed parts.

If it should be part of install then setup.py and requirements.txt should be changed and dropped from the -dev.txt one.

A reno release note should be added that lists these as a new feature addition.

Now I will note that scikit-quant already includes an interop module for qiskit - though because of the recent OptimizerSupportLevel change, that is unfortunately a breaking change if you implemented an optimizer, that interop logic would need revision to be compatible to master. Maybe if the logic is more directly included here then there is no need for an interop module for qiskit in the scikit-quant code - was there any communication with the scikit-quant devs at all?

@woodsp-ibm woodsp-ibm added Changelog: New Feature Include in the Added section of the changelog and removed type: enhancement labels Sep 11, 2020
pbark
pbark previously approved these changes Sep 23, 2020
@woodsp-ibm
Copy link
Member

woodsp-ibm commented Sep 25, 2020

Having just looked at this further the installation of scikit-quant installs the supported set of optimizers from their own packages as dependencies. These dependent packages come in different licenses such as BOBYQA in GPL and SnobFit, ImFil are listed as Other/Proprietary licence. As such I think the scikit-quant should be a purely optional install which means changing imports to check for installation and ensuring that Aqua does not fail to load etc if not installed - same for unit tests they should just skip.

So basically this would go into extra requires in setup.py, and be removed from the requirements-dev. The build can be altered to install these so the function gets tested as is done for other optional installs.

@manoelmarques manoelmarques requested a review from pbark September 26, 2020 18:28
woodsp-ibm
woodsp-ibm previously approved these changes Sep 27, 2020
Copy link
Member

@woodsp-ibm woodsp-ibm left a comment

Choose a reason for hiding this comment

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

@manoelmarques Thanks for taking care of the optional dependence changes.

@manoelmarques manoelmarques merged commit c4cc2a5 into qiskit-community:master Sep 28, 2020
@Brogis1
Copy link
Contributor Author

Brogis1 commented Oct 5, 2020

Great thanks guys ! :)

mtreinish pushed a commit to mtreinish/qiskit-core that referenced this pull request Nov 20, 2020
…/qiskit-aqua#1240)

* snobfit first

* bobyqa optimizer

* imfil optimizer

* orbit optimizer

* orbit added

* snobfit first

* bobyqa optimizer

* imfil optimizer

* orbit optimizer

* orbit added

* snobfit first

* bobyqa optimizer

* imfil optimizer

* orbit optimizer

* orbit added

* final pre-pull

* final pre-pull 2

* added req and docstrings

* copyright corrected

* utf-8 supressed

* missing commas, docstrings.

* make scikit-quant optional

* Add to optimizers documentation

* Spelling: licences (UK variant) to licenses

Co-authored-by: Anton Dekusar <[email protected]>
Co-authored-by: Panagiotis Barkoutsos <[email protected]>
Co-authored-by: Manoel Marques <[email protected]>
Co-authored-by: Steve Wood <[email protected]>
Co-authored-by: woodsp <[email protected]>
manoelmarques added a commit to manoelmarques/qiskit-terra that referenced this pull request Dec 7, 2020
…/qiskit-aqua#1240)

* snobfit first

* bobyqa optimizer

* imfil optimizer

* orbit optimizer

* orbit added

* snobfit first

* bobyqa optimizer

* imfil optimizer

* orbit optimizer

* orbit added

* snobfit first

* bobyqa optimizer

* imfil optimizer

* orbit optimizer

* orbit added

* final pre-pull

* final pre-pull 2

* added req and docstrings

* copyright corrected

* utf-8 supressed

* missing commas, docstrings.

* make scikit-quant optional

* Add to optimizers documentation

* Spelling: licences (UK variant) to licenses

Co-authored-by: Anton Dekusar <[email protected]>
Co-authored-by: Panagiotis Barkoutsos <[email protected]>
Co-authored-by: Manoel Marques <[email protected]>
Co-authored-by: Steve Wood <[email protected]>
Co-authored-by: woodsp <[email protected]>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Changelog: New Feature Include in the Added section of the changelog
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Extend classical optimizers for hybrid quantum/classical calculations
5 participants