UDST / choicemodels

Python library for discrete choice modeling
https://udst.github.io/choicemodels
BSD 3-Clause "New" or "Revised" License
74 stars 33 forks source link

Improve optimization #76

Open sean-wallace opened 1 year ago

sean-wallace commented 1 year ago

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 https://github.com/UDST/urbansim/pull/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?