tulip-control / polytope

Geometric operations on polytopes of any dimension
https://pypi.org/project/polytope
Other
74 stars 19 forks source link

increase the default max_iter in projection_iterhull #30

Open necozay opened 7 years ago

necozay commented 7 years ago

Polytope projection function does not reveal the max_iter parameter of projection_iterhull. I found in playing some problems in tulip with open_loop discretization that projection_iterhull is used due to being efficient for the polytope and projection dimensions considered in such cases however the random initialization fails due to max_iter being small. I often get the error message "Exception: iterative_hull: could not find starting simplex". I think setting max_iter to a larger value (50000?) would be useful. This should not affect the performance of already working examples. Just wanted to check if anyone has any concerns on this before implementing it.

necozay commented 7 years ago

Also mentioned in this tulip issue.

johnyf commented 7 years ago

For now, I support exposing the max_iter value as a keyword argument. It is not necessarily the best design, but until the best design is found, it would be practical. In more detail, to add max_iter to the method Polytope.project, and propagate it to the function projection, which can the pass it on to projection_iterhull.

Having said that, this will be a keyword argument, so the choice of default value still remains. Is there a definite sense of what values are large and small, and what should suffice? (the current default is 10**3). If the value is exposed, and if it is not difficult to pass it in user code, then the default value could be left unchanged.

Another approach (orthogonal to the above) is to add a global constant, which is used as the default keyword argument value (with None, so that updates at runtime be noticed upon calling the function or method). This was done with ABS_TOL in the past. User code can then modify the module parameter once just after importing it, much like one picks a backend for matplotlib.

Even though I do not support global constants much, I think that configuration parameters of solvers are a quite standard use case, and explicit (which is good). A somewhat similar approach is used in dd.cudd, though there one can set parameters through the manager's constructor (__init__), something possible just because there is a "manager" concept, which coincidentally serves also as a context of computation (thus holding parameters like memory bounds, reordering tuning parameters, etc., following CUDD's architecture).

necozay commented 7 years ago

Regarding what value is large enough: for the tulip example we have, I encounter the max_iter bound error approximately seven times out of eight trials. Since randomization is uniform, multiplying max_iter by 8 or 10 would be good. But what is good in general depends on the dimension of the polytope. It just samples until it finds a starting simplex or hits the max_iter bound so having it large will not affect working examples at all since it will keep proceeding with the next step as soon as it finds a solution. The only difference will be: for some examples where it declares the max_iter error and terminates, it will keep searching and it might find a solution and proceed.

If we want tulip example for get_input to run most of the time, setting it to a larger value would be a practical solution. Of course better if we can fix the random number generator too.

johnyf commented 7 years ago

What changes would need to be made to the random number generator?

necozay commented 7 years ago

I meant fixing the seed of the random number generator (see #13).