Refactor OSQP interface into a generic matrix solver #255
Conversation
|
I'll try to go through this soon, but my upcoming week is super full. I would also love to get the CBC interface fixed. Having an open source MILP solver available will add a lot of value (to non-academics like myself). |
|
Yeah sounds good. Feel free to checkout the linked repo with the hybrid solver then. This is a working LP/MIP/QP solver that works for large problems. |
src/optlang/matrix_interface.py
Outdated
| @six.add_metaclass(inheritdocstring) | ||
| class Variable(interface.Variable): | ||
| def __init__(self, name, *args, **kwargs): | ||
| super(Variable, self).__init__(name, **kwargs) |
There was a problem hiding this comment.
Ideally use keyword args only
| super(Variable, self).__init__(name, **kwargs) | |
| super(Variable, self).__init__(name=name, **kwargs) |
src/optlang/matrix_interface.py
Outdated
| _TYPES = ("continuous", "binary", "integer") | ||
|
|
||
|
|
||
| class MatrixProblem(object): |
There was a problem hiding this comment.
Would it make sense to mark this as an abstract class?
| class MatrixProblem(object): | |
| class MatrixProblem(abc.ABC): |
src/optlang/matrix_interface.py
Outdated
| """ | ||
| raise NotImplementedError("This needs to be overwritten by the child class.") | ||
|
|
||
| def clean(self): |
There was a problem hiding this comment.
Not sure that I like the name clean. What about prune or reduce?
src/optlang/matrix_interface.py
Outdated
| but can also be converted to a matrix formulation quickly. | ||
| """ | ||
|
|
||
| def __init__(self): |
There was a problem hiding this comment.
| def __init__(self): | |
| def __init__(self, **kwargs): | |
| super().__init__(**kwargs) |
|
Okay, should have addressed everything. I also removed all the |
There was a problem hiding this comment.
I like the way this is coded and I'm glad to be rid of the use of six. Most of my comments are suggestions only. In principle, it should be possible to upgrade all super calls to empty arguments, as we no longer support 2.7. It can sometimes be tricky with properties, so I'm not sure without trying. I do think it is safer to only use keyword arguments in inheritance.
I did not comment on all places where super is used. Please do a search on the code if you want to change this.
|
Okay should all be adressed now. Some of the super calls needed to stay as is because of the properties like you already suspected. I fixed all the others. |
|
I just noticed that even though the classifiers list Python 3.7 as the minimum version, the actual check is done as: Those should be consistent. I suggest going with I will then update the conda-forge feedstock accordingly. |
|
Done, I also added Python 3.11 to the CI because that was missing as well. |
This is a proposed refactor of the OSQP interface into a generic solver interface for backends that assume immutable problems in (sparse) standard form. This should make it much easier to port solvers to optlang and would for instance be useful to create compatibility with PULP or CVXPY or any other solver that is based on a matrix formulation of the problem. This is also supposed to set the stage for a future HIGHS/OSQP hybrid solver I would like to add.
The interface is still somehwat performant by creating a fast intermediate layer that can be translated into sparse matrices quickly.