Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make API for arbitrary shapes (other than transits) #2

Open
hippke opened this issue Nov 28, 2018 · 11 comments
Open

Make API for arbitrary shapes (other than transits) #2

hippke opened this issue Nov 28, 2018 · 11 comments
Labels
enhancement New feature or request

Comments

@hippke
Copy link
Owner

hippke commented Nov 28, 2018

e.g., flares

@hippke hippke added the performance Make the code run faster label Nov 28, 2018
@hippke hippke added enhancement New feature or request and removed performance Make the code run faster labels Dec 28, 2018
@martindevora
Copy link

Hi. I've already made an implementation for this (based on the exocomets proposal) in my TLS fork. Do you want to check it? Maybe if you found it as a good solution (which might be extensible to any other transit templates), I could create a Pull Request to merge it into the official TLS repository. If you also think that the solutions is not good enough yet, please let me know what additional changes could I do to step forward towards a Pull Request.

Kind regards.

@hippke
Copy link
Owner Author

hippke commented Dec 28, 2020

Cool! I'm very interested in adding this to the main branch of TLS.
Can you elaborate on the features you added, the API, etc? Perhaps even write some documentation on how it works and what it can do? An example to showcase the feature would be cool.

@martindevora
Copy link

martindevora commented Dec 28, 2020

Great. I'll come back with a good explanation, a notebook containing examples and some documentation about the code as soon as I can. Do you have any prefered format for the documentation?

@hippke
Copy link
Owner Author

hippke commented Dec 28, 2020

Wonderful! You could build on the existing documentation (source, ReadTheDocs) and link an iPython notebook?

@martindevora
Copy link

Great. I will do so. I'll let you know when I'm finished.

@martindevora
Copy link

I think that I have added a good start to the documentation of the feature in both places. Please have a look at the notebooks that I added and to the light api changes and let me know whether you think that I should add more info or modify the one I'm providing.

In general terms, I have moved the template related logic to a class named TransitTemplateGenerator, which is implemented by DefaultTransitTemplateGenerator for planetary transits. There is also CometTransitTemplateGenerator now. Finally, the user can decide to provide his custom implementation of TransitTemplateGenerator to search for custom templates. You can see in the last notebook that I tried a template for flares.

I had to modify tls logic which assumed that the templates only caused flux drops, and now flux increments would be supported.

The main issue I'm concerned about right now is the transit depth adjustment, which can be inexact for several reasons. One of them is the overshoot parameter, whose meaning I dont understand. I think that you created an issue to provide depth fits instead of an analytical one and that might be related too. However I think that the solution works properly enough now and the things that I'm commenting here could probably be developed in the future.

Kind regards.

@martindevora
Copy link

martindevora commented Dec 31, 2020

As soon as I can I will create some tests and will open a pull request so you can better inspect the changes.

@hippke
Copy link
Owner Author

hippke commented Jan 23, 2021

Sorry @martindevora again for the delay. Schools are still closed which leaves me hardly any time. I hope the situation will get better in February. Current estimate is either 1 Feb or 15 Feb for re-opening.

@martindevora
Copy link

There is no hurry at all @hippke . I'm adding a few improvements into the new API so we can have a more maintainable code. The most important thing right now is to keep your family and yourself safe and healthy. I'm doing as well. I would probably like to use these changes as a base to develop some kind of publication at some point in the future and I would like to know what you think about it.

As already said, there is no hurry, so don't worry about this until you feel like you can do it happily without pressure.
Kind regards,
Martín.

@hippke
Copy link
Owner Author

hippke commented Jan 31, 2021

I think it's a great idea worth implementing in code, and worth searching for in real data. I'm not a specialst of flare-finding algorithms, but the few I have seen are rather simple. So it appears useful to try another approach.
I'm a bit sceptical with regards to periodicity. TLS has built-in phase-folding, so that's sort of given. Many flares and comets might be somewhat periodic, e.g. due to stellar rotation, but in many cases not perfectly periodic. As you have shown in your example, TLS can still find such things, but the sensitivity will be reduced. If you want to find stuff with it, it might work! But you should compare sensitivities to other algorithms out there, to check in which use cases TLS+mod is good, and in which it is bad.

@martindevora
Copy link

Yes totally. I'm planning to compare TLS against such other algorithms (with its improvements and counterparts) and with the standard transit models in case of tailed transits.

I was also wondering in using a final_fit method within TLS to add the option to the user of adding a complete custom fit for the given template, which could be a scipy.curve_fit for instance (or whatever the user would like to introduce). What do you think about it? I guess that it is not strictly necessary as it can be done after TLS returns the results, but it would be a way to unify the final fit processing for all the search algorithms.

Kind regards.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants