WIP: Approximate convex hulls #114
Conversation
1 similar comment
| try: | ||
| import cvxopt | ||
| import cvxopt.solvers | ||
| except: |
There was a problem hiding this comment.
Should only catch on ImportError
There was a problem hiding this comment.
Probably, though I don't want any error in importing CVXOPT to affect functions which don't require it. Perhaps silently ignoring ImportError and raising a warning on other exceptions would be good?
There was a problem hiding this comment.
I would catch on ImportError and let all others raise an issue. This is important because if you have code that can use cvxopt, and the package is installed, but something goes wrong (maybe an environment issue, or windows being stupid, etc) that it should halt.
If you want more rigorous separation you'll either need to use separate files (one for functions that use cvxopt and one for those that don't), or if the number of functions that require this package is low (but those functions in turn do require them) you can lazy-load the package by doing your import statement in the function itself. Keep in mind though that that will be called every time the function is called, and while it is fast, its not free.
|
|
||
| n_partial, dimension = partial_hull.shape | ||
| n_candidates, dimension_check = candidate_points.shape | ||
| assert dimension == dimension_check |
There was a problem hiding this comment.
This should probably raise something instead of just blind asserting
There was a problem hiding this comment.
Absolutely. Given that this is still a first stab, I didn't yet take the care to validate inputs and all that will go into the final PR.
| @@ -0,0 +1,279 @@ | |||
| #!/usr/bin/python | |||
There was a problem hiding this comment.
Good catch, this is what I get from copying a older header template, heh.
This work in progress PR aims to produce ε-apporixmate convex hulls using the algorithm of 1603.04422, with CVXOPT being used to solve the quadratic program at the innermost loop. Unfortunately, this approach results in CVXOPT taking up a truly absurd ratio of the runtime, so the current implementation is far too slow to be useful at the moment. That said, it seems to work reasonably well in local testing, even if its too slow to be usable.