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

Improve optimization #76

Open
wants to merge 5 commits into
base: dev
Choose a base branch
from

Conversation

sean-wallace
Copy link

Hello @smmaurer and perhaps others. My team is using choicemodels to estimate some relatively large location choice models in a land use model setting and we have been having a lot of problems with convergence (using engine == "ChoiceModels"). In searching for suggestions on what we might be doing wrong, I've noticed a few similar issues around, notably the one at UDST/urbansim#212 from 2018.

I think I might have found the culprit... Please feel free to ignore the first commit here, I'm not sure that it is material at all though it did initially seem to allow some models to converge which were failing with strange errors before. I think in particular the value of factr is set to be too small when calling scipy.optimize.fmin_l_bfgs_b()

The iteration stops when (f^k - f^{k+1})/max{|f^k|,|f^{k+1}|,1} <= factr * eps, where eps is the machine precision, which is automatically generated by the code. Typical values for factr are: 1e12 for low accuracy; 1e7 for moderate accuracy; 10.0 for extremely high accuracy.

I don't know what a minimum value for factr might be to achieve "extremely high accuracy" but I think machine precision times ten might be too high of a bar. Changing this to factr=10e9 gets reasonably close to the tolerances being used in pylogit (1e-06) once divided by eps. The implementation of scipy.optimize.minimize() that pylogit uses internally calls the same function that you are using here.

Along with loosening this tolerance, I think another improvement might be to move to using that scipy.optimimze.minimize() interface here in choicemodels. I did see a comment somewhere in a scipy issue that directly calling the optimizers themselves should be considered deprecated. I'd be happy to tackle that change if you'd be interested in merging.

What do you think?

@sean-wallace sean-wallace marked this pull request as ready for review January 26, 2023 02:03
@sean-wallace sean-wallace force-pushed the improve-optimization branch 2 times, most recently from 8c704ed to 791b1cc Compare August 31, 2023 16:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant