search

ekaterinailin / AltaiPony

Find flares in Kepler and TESS light curves. Notebooks for quickstart inside.
https://altaipony.readthedocs.io
MIT License
26 stars 10 forks source link

New flare model from Tovar Mendoza et al. (2022) #75

Closed lupitatovar closed 1 year ago

lupitatovar commented 1 year ago

I added in the new Tovar Mendoza et al. (2022) flare model.

ekaterinailin commented 1 year ago

@lupitatovar Awesome, thanks for opening the pull request! Two things I'd like you to adapt before can merge your new version in:

  1. At least one test fails because the module special is missing. Can you please add it to the preamble and requirements.txt if necessary?
  2. I saw you replaced the test for a aflare with the same test for your new flare_model. I'd like to keep testing the aflare, and still offer it to users who want to make a comparison with the new one. Could you please make a copy of test_aflare_and_equivalent_duration, so that one copy tests aflare and the other tests flare_model?

Once that is done, we shall check if the remaining tests run through, or if we need to tinker a little more to make it work.

Again, great job opening the pull request, and also adding the docs!

lupitatovar commented 1 year ago

@ekaterinailin I just added in those two things. Let me know if there are more things we need to add/fix. Thanks for your help with incorporating in the new model!

ekaterinailin commented 1 year ago

Hi @lupitatovar !

Thanks for making the changes, looks good! The test_flare_model() does not yet work, however. Does the test pass on your version?

In the continuous integration, the test still fails, see here. Can you adapt the test for flare_model? So far, it looks like it's just a copy of test_aflare.

lupitatovar commented 1 year ago

Hi @ekaterinailin! I adapted the test and got things to pass on my end. I also caught a bug in the upsample feature of flare_model and fixed that as well. Let me know how things look on your end. Thanks!

ekaterinailin commented 1 year ago

Looks good to me, thanks for updating the pull request! I will make sure, that the new flare model is accessible in FlareLightCurve.