Closed AmitSolomonPrinceton closed 6 months ago
Great to see that the CI is working on this PR!
I have a couple of suggestions, mostly having to do with my taking a shortcut on this before, something that might be handled better with this PR (and going forward):
torch
as a package dependency was that its a heavy and complex dependency, probably applicable to a small majority of osqp
users. Users of the nn
submodule may as well see an ImportError
on torch
and know what they're supposed to do. Now that we have joblib
joining that list as well. it may be better something like a [nn]
extra so that pip install osqp[nn]
installs the dependencies required for the nn
functionality.joblib
is a small and generally useful library, it can be introduced as a direct dependency, and torch
left out as before.CIBW_TEST_REQUIRES
will keep things manageable going forward. (and the CI can simply do a install osqp[dev,nn,..]
etc.I also don't recommend including .devcontainer
in this PR, since that might trigger an IDE-specific proliferation of code, but that's just my personal opinion.
Improvements to torch.py, including parallelization, refactoring, update existing solvers instead of recreating them. Also added devcontainer, set the version of scipy to <1.12, and update build files.