MNGuenther / allesfitter

allesfitter is a convenient wrapper around the packages ellc (light curve and RV models), dynesty (static and dynamic nested sampling) emcee (Markov Chain Monte Carlo sampling) and celerite (Gaussian Process models).
MIT License
62 stars 36 forks source link

[Bug] show_initial_guess exits ungracefully #52

Closed martindevora closed 2 years ago

martindevora commented 2 years ago

Hi, for the newer version of matplotlib 3.5.1 the show_initial_guess method is failing when calling plt.savefig, maybe due to some issue with memory handling. I attach my allesfitter directory.

fit_0.zip

Regards.

martindevora commented 2 years ago

I've tried also matplotlib 3.5.2 and the issue still happens. For these kind of huge plots (many transits for individual transits plot) you would probably interested into storing individual images for each transit (or groups of three transits, for instance) and then create a PDF with all of them using something like ReportLab. Another option is to create n plots for each group of (let's say) 20 transits.

martindevora commented 2 years ago

In case you are interested, I can upload a Pull Request to implement this: Another option is to create n plots for each group of (let's say) 20 transits.

martindevora commented 2 years ago

I'd created a Pull Request here https://github.com/MNGuenther/allesfitter/pull/53

Please let me know if you'd like to improve anything in the solution.

MNGuenther commented 2 years ago

This is fantastic, thanks @martindevora! I have just merged the pull request!

Before we can close this: I think we might need to add that same "splitting" feature also into the prepare_ttv_fit plots of the individual transits, right?

I have just cleaned up the prepare_ttv_fit script to (hopefully) resolve the other issues you had raised. Would you mind implementing your "splitting" feature in there, too (whenever you encounter it necessary)?

martindevora commented 2 years ago

Hi. Yes I'll do it as soon as I can. I'll keep you posted.

martindevora commented 2 years ago

Hi @MNGuenther I've done it in this Pull Request https://github.com/MNGuenther/allesfitter/pull/54. I did some simple tests locally but I'd advice you to try it out too. Regards.

MNGuenther commented 2 years ago

Thanks @martindevora, merged!