search

ExcitedStates / qfit-3.0

qFit: Automated and unbiased multi-conformer models from X-ray and EM maps.
MIT License
31 stars 11 forks source link

Improve implementation of open solvers for SBGrid #374

Closed stephaniewankowicz closed 2 months ago

stephaniewankowicz commented 7 months ago

We want a set of working open solvers to be able to put qFit into SBGrid

blake-riley commented 7 months ago

IMO, this was done in #363, which reimplemented cvxopt (QP), osqp (QP) and miosqp (MIQP). The choice can be made with --qp-solver and --miqp-solver respectively.

Or is this just an issue to track until dev is merged with main?

stephaniewankowicz commented 7 months ago

The open MIQP solver does not work currently (https://github.com/osqp/miosqp)

blake-riley commented 7 months ago

It crashes? Doesn't load? Can you give me a crash log, and I'll look into it tonight?

stephaniewankowicz commented 7 months ago

This issue is with the following line:

class MIOSQPSolver(MIQPSolver): driver_pkg_name = "miosqp" driver = lazy_load_module_if_available(driver_pkg_name) if TYPE_CHECKING: import miosqp

The import fails. This package is not included in osqp. (https://github.com/osqp/miosqp)

blake-riley commented 7 months ago

That's correct. miosqp is a separate package. Unfortunately though, the authors haven't distributed it in the pip repositories, only on GitHub.

I would guess that your issue is because miosqp isn't installing with qfit.

It's not installing for 2 reasons:

  1. I made a mistake in setup.py. Where it says extra_dependencies, it should read extras_require.
  2. Even if that was fixed: it's not obvious, but when you install qfit with pip install . (or pip install qfit one day, I guess), you'd have to specify the set of qfit "extensions" you'd like in square-brackets (pip install .[osqp] or pip install .[cplex,osqp]).[^cplex]

Proposed fix Since we need to have at least one set of solvers, we should probably make the osqp solvers install by default, and just include them straight up in the install_requires. This would likely satisfy SBGrid.

I will put in a PR to fix this tonight.

[^cplex]: Even then, cplex is only partially available through pip, so this only really works on old macs, or on linux machines.

stephaniewankowicz commented 2 months ago

Completed in a different PR