johnnygreco / pymfit

python-imfit
MIT License
4 stars 1 forks source link

getting imfit-mcmc working #1

Closed johnnygreco closed 7 years ago

johnnygreco commented 7 years ago

Currently, in pymfit.run, mcmc_prefix is supposed to be the --output option for imfit-mcmc, but it isn't implemented yet; I'll fix this soon. In addition, pymfit.run automatically reads the results assuming a single Sersic fit using imfit rather than imfit-mcmc, but I don't think imfit-mcmc generates a best-fit file, so the function breaks at this point. For now, I'll just return None if mcmc=True, but maybe we'll think of a more useful option later.

I'm also noticing an awful lot of input parameters for this function... not sure if we should even worry about trying to make it cleaner at the moment, but I thought I'd mention it in case you have any ideas about how to clean it up a bit.

dr-guangtou commented 7 years ago

Great! I think now the main reason I want to try MCMC is to have a decent error bar for magnitude and size.

About other input options, I identify a few potentially useful ones. I agree that none of them is urgent now.

     --nchains <int>              Number of separate MCMC chains [default = # free parameters in model]
     --max-chain-length <int>     Maximum number of likelihood evaluations per chain [default = 100000]
     --burnin-length <int>        Number of generations in burn-in phase [default = 5000]
     --gelman-evals <int>         Perform Gelman-Rubin convergence check every N generations [default = 5000]
     --gelman-rubin-limit <float> Gelman-Rubin scale reduction factor limit [default = 1.01])
     --uniform-offset <float>     MCMC uniform-offset term [boundary for uniform offsets of scaling; default = 0.01]
     --gaussian-offset <float>    MCMC b^star term [sigma for absolute Gaussian offsets; default = 1.0e-6]
     --cashstat                  Use Cash statistic instead of chi^2
     --poisson-mlr            Use Poisson maximum-likelihood-ratio statistic instead of chi^2
johnnygreco commented 7 years ago

I've added these updates, which you can see in commit 98ee927.

For now, I created an mcmc_kws argument for all the parameters you listed. So, you can do something like:

# note you can't use dict() due to the hyphens
# in the imfit option names
mcmc_kws = {'nchains': 8, 'max-chain-length': 10000} 
pymfit.run(...input params..., mcmc_kws=mcmc_kws)

Note that imfit-mcmc seems to be unbelievably (see the mcmc-explore.ipynb notebook I added) slow on my machine. Let me know how it works for you.

dr-guangtou commented 7 years ago

Great! Looks good. I'll try it during the weekend.

3min does not sound very bad to me....I once tried MCMC Sersic fitting by myself, it is much slower...

BTW: I learn that if you include something like: "Fix #1" in your commit message, it will link the commit to this issue, and show all the file differences here.

johnnygreco commented 7 years ago

It actually took 2 hrs! The 3 minutes was for bootstrap.

Thanks for the commit message tip!

johnnygreco commented 7 years ago

I thinks this is working now... I'll close the issue.