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

Wrong estimate for transit duration if long gaps are present #83

Open
hippke opened this issue May 14, 2020 · 8 comments
Open

Wrong estimate for transit duration if long gaps are present #83

hippke opened this issue May 14, 2020 · 8 comments
Labels
bug Something isn't working

Comments

@hippke
Copy link
Owner

hippke commented May 14, 2020

e.g., when combining two TESS sectors with a long gap in between: Transit duration estimate is wrong.

@hippke hippke added the bug Something isn't working label May 14, 2020
@hippke
Copy link
Owner Author

hippke commented Jun 4, 2020

Suggested solution by @MNGuenther

ind_tr_phase = np.where( results['model_folded_model'] < 1. )[0]
lc_duration = results['period'] * (results['model_folded_phase'][ind_tr_phase[-1]] - results['model_folded_phase'][ind_tr_phase[0]])

@martindevora
Copy link

Hi. Is this planned to be solved any time soon? I'm finding the same issue. The transit duration value is different than the duration calculated from the model_folded_phase, which is the one looking correct.

Kind regards.

@mkunimoto
Copy link

While the above suggested fix correctly computes the transit duration from the best-fit transit model, I find that the best-fit transit model itself doesn't seem to be a good match when long gaps are present. Likewise, the estimated S/N (an important value for planet detection criteria) is much lower than one would expect.

As an example, here are the results for TIC-352682207 (TOI-4010), using a multi-sector TESS light curve (sectors 24, 25, 52, and 58). Using just sectors 24 and 25, TLS gives a good fit to the data and finds a duration of 2.1 hours (a little shorter than the expected 2.8 hours, but somewhat expected given this is 30-minute data) and S/N = 18.4:

Screen Shot 2023-08-14 at 2 39 00 PM

And TLS shows a good fit when searching only sector 58, recovering a 2.8-hour duration (S/N = 12.9):

Screen Shot 2023-08-14 at 2 40 51 PM

But when searching all four available sectors, the model is visibly worse. The model has only a 1.6-hour duration and the S/N is now 9.8:

Screen Shot 2023-08-14 at 2 41 42 PM

@martindevora
Copy link

Hello @mkunimoto The problem comes when tls computes the intransit and out of transit points due to an estimated transit_duration_in_days using this method. When there are gaps in the curve, the inner calculate_stretch method returns wrong values that lead to smaller transit durations and hence, underestimated SNRs. In my case, I created a fork which I'm also publishing in pypi named foldedleastsquares where I re-compute the transit duration as done here. That way the SNR scales accordingly to the expected additional provided data. The API is the same as the one offered by tls with some small additions.

If my workaround was not found to be wrong, I'd be glad to request a merge of my fork to the official repository but I tried it some time ago and it hung on a pending pull request for several months; therefore I retired it.

Regards.

@hippke
Copy link
Owner Author

hippke commented Aug 14, 2023

Hey both, sorry for the bug and sorry for not merging the pull-request. Not sure what went wrong back then. Happy to merge - how do we do this? Do you want to resubmit?

@martindevora
Copy link

My fork is also including a medium-sized code refactor to allow different transit templates to be applied and it grew in changes with time. Thus, I think that I could try to create a pull request from a simpler fork with only the changes mentioned in this issue or you could take a look at the changes I pointed out in my previous comment and cherry-pick them into the official project if you find them correct.

@hippke
Copy link
Owner Author

hippke commented Aug 14, 2023

If possible I'd prefer just a bugfix for the issue mentioned here. I'd merge that and release a new version. So there is minimum risk for anything else to break.
The new features should be a subsequent change, and I'd want to do some tests on my own for them.

@mkunimoto
Copy link

Thanks @martindevora and @hippke - will the bugfix just fix the duration estimate? Or will it also address the transit model not fitting well for multi-year observations?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants