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

docplex optimizers #57

Merged
merged 14 commits into from
Jul 18, 2022
Merged

docplex optimizers #57

merged 14 commits into from
Jul 18, 2022

Conversation

gcattan
Copy link
Collaborator

@gcattan gcattan commented Jun 25, 2022

Follow up for #28

This PR introduces one classical and one naive quantum optimizer, as per comment #56 (comment)
It also bump the version of docplex and add a new dependency to qiskit_optimization.

@gcattan gcattan marked this pull request as ready for review June 25, 2022 20:27
@toncho11
Copy link
Collaborator

toncho11 commented Jul 17, 2022

I am looking into the test program. There are actually no comments in it.

  1. It looks like a docplex problem is specified with "test_get_square_cont_var", but then the optimizer is created without providing this docplex model? I do not understand what the "test_get_square_cont_var" is doing.

  2. Can we also call "solve()" on the two optimizers (quantum and classical) and validate that a result has been returned? Or it is too early for that?

For me the workflow (with comments) should be:

  • define a docplex model
  • select optimizer
  • provide the docplex model to the optimizer (or several optimizes)
  • call "solve" on the optimizer (or on several optimizes)

add comments to `test_get_square_cont_var`
@gcattan
Copy link
Collaborator Author

gcattan commented Jul 17, 2022

Hi,

I have added comments to "test_get_square_cont_var", I hope it is more clear.

As for the functional tests of the solve method of the optimizers themselves, I would prefer to do it along with #26 to reuse the docplex model that would be defined in #26. What do you think?

@toncho11
Copy link
Collaborator

toncho11 commented Jul 18, 2022

Maybe you can add a general description of "docplex.py " such as:
"This module contains both classic and quantum optimizers, and some helper functions. The quantum optimizer allows an optimization problem with constraints (in the form of docplex model) to be run on a quantum computer. It can be used for ... "

I think some more general description will be quite helpful.

Add description of module docplex.ply

Co-authored-by: toncho11 <toncho11@gmail.com>
@gcattan gcattan merged commit 84e4d5f into pyRiemann:main Jul 18, 2022
@gcattan
Copy link
Collaborator Author

gcattan commented Jul 18, 2022

Thx for the review @toncho11 :)

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.

2 participants