Addresses problems with find_MAP and discrete stochastic variables
#412
Conversation
This modifies the find_MAP function to better handle discrete variables. This is accomplished in three ways. 1. Warning the user when discrete variables are used that MAP estimates may be inaccurate 2. Using `fmin_powell` instead of `fmin_bfgs` as this seems to be more likely to converge on actual values 3. Convert discrete values to their floor after minimization to reflect the support of the underlying discrete variables.
Fixes overwrite of `fmin` whenever discrete variables are present
Suppress the warning when a `fmin` is provided by the user, this removes the warning message (which is somewhat annoying), in the case where it seems like the user knows what she is doing.
Me fail english, that's unpossible!
fmin was set to the default value inefficiently, it is now more efficient
|
I can additionally add a test, and possibly an example. What is the policy regarding examples in |
| @@ -25,8 +26,13 @@ def find_MAP(start=None, vars=None, fmin=optimize.fmin_bfgs, return_raw=False, | |||
| start : dict of parameter values (Defaults to model.test_point) | |||
| vars : list | |||
| List of variables to set to MAP point (Defaults to all continuous). | |||
| If discrete variables are included, estimates are likely to be | |||
There was a problem hiding this comment.
I'm not sure we need this, since the warning will make this clear if it comes up.
|
Looking good! This will alert users to when discrete variables can lead to unexpected behavior, as well as making it easy to optimize over discrete variables, which we didn't have before. I wonder if we should default to optimizing over the discrete variables now? Does powell usually work quite well? Definitely would be nice to have a test. My preference here would be for a .py example. My thinking is that .ipynb's are flashier, prettier and better with graphs, but a little more cumbersome than .py's, so mostly for tutorials or things that really benefit from graphs and .pys for the rest. If we have a .ipynb, we should also have a .py. |
|
@aflaxman pointed out once that powell works quite well for finding the MAP in pymc 2. We've been using it ever since and of the gradient-free it seems to do the best. |
Updates the documentation and displays in `find_MAP` a warning only when `disp=True` and discrete variables are specified.
Accidentally overrode fmin when specified
Added a test that tests the results of using `find_MAP` on models with discrete stochastics.
Removed some extraneous warnings in discrete_find_MAP.ipynb that were due to my particular environment.
|
I think I addressed the above concerns. It might be difficult to merge with #411 as @jsalvatier has made some useful changes to the warning messages. |
|
Looks great! Thanks for this. |
Addresses problems with `find_MAP` and discrete stochastic variables
Addresses #396. This pull request addresses some issues with finding Maximum a Posterior(MAP) estimates when a model contains discrete stochastics.
fmin_powell) when discrete stochastics are used.fminis provided andvarscontains discrete stochastics, provide a warning, and switch minimization methods