Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

increase the default max_iter in projection_iterhull #30

Open
necozay opened this issue Nov 4, 2016 · 5 comments
Open

increase the default max_iter in projection_iterhull #30

necozay opened this issue Nov 4, 2016 · 5 comments
Labels
enhancement A new feature, an improvement, or other addition.

Comments

@necozay
Copy link
Contributor

necozay commented Nov 4, 2016

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 necozay added the enhancement A new feature, an improvement, or other addition. label Nov 4, 2016
@necozay
Copy link
Contributor Author

necozay commented Nov 4, 2016

Also mentioned in this tulip issue.

@johnyf
Copy link
Member

johnyf commented Nov 5, 2016

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
Copy link
Contributor Author

necozay commented Nov 10, 2016

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
Copy link
Member

johnyf commented Nov 10, 2016

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

@necozay
Copy link
Contributor Author

necozay commented Nov 11, 2016

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement A new feature, an improvement, or other addition.
Projects
None yet
Development

No branches or pull requests

2 participants