Skip to content

[WIP] Introduce a framework to convert a modeling library to a quadratic program and vice versa#116

Closed
t-imamichi wants to merge 46 commits intoqiskit-community:mainfrom
t-imamichi:translator
Closed

[WIP] Introduce a framework to convert a modeling library to a quadratic program and vice versa#116
t-imamichi wants to merge 46 commits intoqiskit-community:mainfrom
t-imamichi:translator

Conversation

@t-imamichi
Copy link
Copy Markdown
Collaborator

@t-imamichi t-imamichi commented Apr 30, 2021

Summary

It introduces a model translator to convert a modeling libraries (e.g., docplex, gurobipy, etc.) to QuadraticProgram and vice versa.
It adds QuadraticProgram.from_model(model, translator) and QuadraticProgram.to_model(translator) as a generic version of from_docplex and to_docplex.

I revised the design in #122 without gurobipy.

Fixes #80

TODO:

  • Design discussion
  • Fix cyclic imports

Details and comments

@CLAassistant
Copy link
Copy Markdown

CLAassistant commented Apr 30, 2021

CLA assistant check
All committers have signed the CLA.

@t-imamichi t-imamichi changed the title Introduce a framework to convert a modeling library to a quadratic program and vice versa [WIP] Introduce a framework to convert a modeling library to a quadratic program and vice versa Apr 30, 2021
Comment on lines -814 to -873
def from_docplex(self, model: Model) -> None:
"""Loads this quadratic program from a docplex model.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Should we deprecate from_docplex as well as to_docplex?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Comment on lines +42 to +44
"""

def qp_to_model(self, quadratic_program: Any) -> GurobiModel:
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Perhaps quadratic_program is an instance of QuadraticProgram, not Any?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

It is impossible to avoid cyclic import by pylint if quadratic_program: QuadraticProgram. If we are allowed to disable pylint cyclic import, I can do it.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Oh, I see the problem now.

Comment on lines +127 to +130

return mdl

def model_to_qp(self, model: Model, quadratic_program: Any):
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

We could avoid having a quadratic_program as a method parameter if we made from_model static.
And should we have correct type hints?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Yes, making from_model static is an option.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

should just return a QuatraticProgram, doesn't need to be static for that (and I'd keep it non-static, maybe there are cases where we would like to parametrize the translator). Same for GUROBI and interface.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@stefan-woerner I mean QuadraticProgram.from_model() to be static, not the methods in the translators.
I think it should look like: qp = QuadraticProgram.from_model(another_model, translator)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@adekusar-drl thanks, sounds good!

Comment on lines +817 to +820
if translator is None:
from qiskit_optimization.translators.docplex import DocplexTranslator
translator = DocplexTranslator()
translator.model_to_qp(model, self)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Should we have a translator instance explicitly instead of defaulting to DOcplex?

Comment on lines -814 to +803
def from_docplex(self, model: Model) -> None:
"""Loads this quadratic program from a docplex model.
def from_model(self, model: Model, translator: Optional[ModelTranslator] = None) -> None:
"""Loads this quadratic program from an optimization model.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Is model: Model valid? I guess Model here comes from DOcplex? Should it be something more generic?

I suggest to make from_model a static method to be able to create a model like qp = QuadraticProgram.from_model(another_model, translator)

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

You are right. model: Model is invalid here because Model is docplex.mp.model.Model. I agree to making from_model static.

Comment on lines +40 to +42
"""

def qp_to_model(self, quadratic_program: Any) -> Any:
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Should we have correct type hints?

Copy link
Copy Markdown
Collaborator Author

@t-imamichi t-imamichi May 3, 2021

Choose a reason for hiding this comment

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

It is impossible due to pylint outputs cyclic import errors. pylint does not seem to support typing.TYPE_CHECKING. So, I cannot find a way to avoid cyclic import errors except disabling it. Do you have any good idea?

Comment on lines +21 to +24
"""

@abstractmethod
def qp_to_model(self, quadratic_program: Any) -> Any:
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Same as above, why quadratic_program: Any, but not quadratic_program: QuadraticProgram ?

Comment on lines +32 to +36
"""
pass

@abstractmethod
def model_to_qp(self, model: Any, quadratic_program: Any) -> None:
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Type hints

t-imamichi and others added 4 commits May 6, 2021 19:30
# Conflicts:
#	qiskit_optimization/algorithms/__init__.py
#	qiskit_optimization/applications/max_cut.py
#	qiskit_optimization/problems/quadratic_program.py
#	test/problems/test_quadratic_program.py
# Conflicts:
#	qiskit_optimization/problems/quadratic_program.py
Comment on lines +127 to +130

return mdl

def model_to_qp(self, model: Model, quadratic_program: Any):
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

should just return a QuatraticProgram, doesn't need to be static for that (and I'd keep it non-static, maybe there are cases where we would like to parametrize the translator). Same for GUROBI and interface.

class GurobipyTranslator(ModelTranslator):
"""Translator between a gurobipy model and a quadratic program"""

def qp_to_model(self, quadratic_program: Any) -> GurobiModel:
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Type hint: quadratic_program be of type QuadraticProgram (see @adekusar-drl's comments on interface and GUROBI as well)

@t-imamichi
Copy link
Copy Markdown
Collaborator Author

Close it to continue discussion at #122

@t-imamichi t-imamichi closed this Jun 1, 2021
@t-imamichi t-imamichi deleted the translator branch June 8, 2021 02:24
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.

General conversion interface between QuadraticProgram and modeling libraries

6 participants