search

replicahq / doppelganger

A Python package of tools to support population synthesizers
Apache License 2.0
165 stars 32 forks source link

Allow users to specify which CVX solver they would like to use... #32

Closed nikisix closed 7 years ago

nikisix commented 7 years ago

for household allocations.

Non-essential of course, but I had to do this yesterday as part of my debugging process so I figured I'd offer it up.

This change is Reviewable

alexeisw commented 7 years ago

Thanks for the PR and a suggestion!

The idea in the implementation of the allocation algorithm was to hide all these practicalities from the users instead of exposing them with a choice to keep trying out different solvers until one converges. The logic was to keep changing the relaxation constraint and finally, if nothing helps, to fall back on (proportionally) initial weights with logging a warning. I think a little deeper analysis of the corner cases when this logic does not work would be great. It might be that changing a solver can be a part of the strategy to resolve convergence issue automatically, or, better yet, using some diagnostics that will guide the appropriate choice of the solver to try in the first place. What do you think?

Nits: the coding style for this repo prescribes 100 characters max line length, please keep an eye on it. Current build fails due to linter warning of >100 lines.

Review status: 0 of 2 files reviewed at latest revision, all discussions resolved, some commit checks failed.

Comments from Reviewable

nikisix commented 7 years ago

@alexeisw I think you're right that deeper corner-case analysis may be warranted. From my brief reading over the past few days of the CVX project their default solver ECOS is quite good, but we might be able to do a better job of catching when it's not converging b/c I've hit their solver with a few cvx-problems that can cause it to spin for hours, then fail. It's something @kaelgreco and I are planning on looking more into I believe.

Additionally, the (open-source) solver ordering their docs suggest is something like ECOS, CVXOPT, SCS (fast and converges a wide problem set, but comparatively inaccurate). I definitely see where you're coming from of hiding the solver, and controlling relaxation constraints and solver choice internally -- something very few of our users will want to wrestle with most likely.