Re-implement open source solvers #363
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
blake-riley
force-pushed
the
feature/361-open-source-solvers
branch
from
8f05714
to
789cb4b
Compare
|
Yeah, we're failing tests. A few things to work on:
|
natechols
reviewed
Sep 18, 2023
natechols
reviewed
Sep 18, 2023
blake-riley
force-pushed
the
feature/361-open-source-solvers
branch
from
789cb4b
to
991facc
Compare
The advantage of an initialize() method is to pre-cache an expensive operation.
For QPSolver:
QPSolvers are always instantiated, then solved immediately.
There's no caching going on here.
For MIQPSolver:
In BIC threshold loops, MIQP solver.solve is run a few times.
The initialize method can either
a. be made private here, since it saves us N loop iterations * 2 inner products
b. be thrown away, since these inner products are pretty cheap as
there aren't many conformers for MIQP
I've decided on (a), as I don't know the performance improvement of
pre-caching here, and maybe later on MIQP will be done in situations
with lots of conformers?
blake-riley
force-pushed
the
feature/361-open-source-solvers
branch
2 times, most recently
from
9260310
to
bd1666e
Compare
|
Awesome. We're passing now. |
…lable solvers to user. **Single responsibility** - The solvers.py module shouldn't be responsible / shouldn't contain the logic for deciding _which_ solver to use. This should be left up to the invocation script (e.g. qfit_protein.py). - We should however, guarantee that all Solvers will conform to a particular interface. solvers.py is thus an _adapter_ module between a variety of solver packages and the common interface we define here. **importlib hooks, lazy module loading** (utils/optional_lazy_import.py) - We will provide some helper functions to allow the invocation scripts to know what solvers are available (and present them as options to the user). - Each solver must try to import its own 'driver' module (e.g. cplex, cvxopt). This is _modular_. - We also want to have a lazy import: modules are not executed when they are imported, only on first property call. Without a LazyLoader, since all solver 'driver' modules are imported during creating the argparse options, they would be eagerly executed, taking a lot of time for the user. With a LazyLoader, while the driver modules have been "imported", we delay their code execution until when they are first used.
This rolls back commit a2bba46, and updates it for the current architecture.
If the user installs with `pip install qfit[cplex]`, they get the cvxopt and cplex solver modules. `pip install qfit[osqp]` installs the osqp + miosqp solver modules.
All SciPy subpackages need to be imported separately (https://docs.scipy.org/doc/scipy/tutorial/index.html)
This is required to get pytest to test these modules
This test actually runs a QFitLigand job, so needs to know which solver to use. WARNING: test_qfit_protein.py does not actually run a QFitProtein job, so it doesn't need this same mod. It should be re-written.
blake-riley
force-pushed
the
feature/361-open-source-solvers
branch
from
bd1666e
to
3adcc6f
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Pull Request Checklist
devbranch?Exceptions will be made for urgent bugfixes.
dev?If not, please rebase your PR onto the most recent
devtip.Explain to a new user by completing the sentence: 'This PR will: ...'
Description of the Change
Fixes #361 .
Release Notes
qfit_protein --help--qp-solver=CVXOPTSolver --miqp-solver=MIOSQPSolver)