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

Support for transit duration variations #73

Open
dfm opened this issue Jan 16, 2020 · 8 comments
Open

Support for transit duration variations #73

dfm opened this issue Jan 16, 2020 · 8 comments
Labels
enhancement New feature or request

Comments

@dfm
Copy link
Member

dfm commented Jan 16, 2020

A feature request from @gjgilbert.

A more general interface could be to allow any of the orbital elements to be a function of transit number. Right now, transit time is hard coded as the only one to vary, but it shouldn't be too bad to add impact parameter (or inclination? or both? things start to get a little tricky) as another parameter that varies. I'm inclined to think that the more general interface might be easier to maintain in the long run, but I'm open to suggestions!

@dfm dfm added the enhancement New feature or request label Jan 16, 2020
@gjgilbert
Copy link
Contributor

Thanks for considering this, @dfm.

I can think of two ways I'd want to interact with this feature as a user:

  1. Warping of the transit shape in a way that is not necessarily tied to physical orbital elements. That is, transit time variations, transit duration variations, and transit depth variations.

  2. Variable orbital elements, e.g. impact parameter, eccentricity, etc. that are more closely tied to physically relevant dynamics.

I'm not sure which would be easier to implement/maintain, but I can think of good scientific reasons to want both functionalities. TTVs are nice in that they work equally well under either of the two approaches. From an end-user perspective, a general interface would be very much appreciated!

@dfm
Copy link
Member Author

dfm commented Jan 16, 2020

For the 1st use case, folks should generalize the SimpleTransitOrbit implementation instead of the KeplerianOrbit or TTVOrbit. I'm not excited about implementing that, but I would definitely be keen to chat about a pull request!

@gjgilbert
Copy link
Contributor

Yes, let's chat about a pull request! The 1st use case is more suited for my current project, so I'd be excited to work on it.

@ericagol
Copy link
Collaborator

I think 1. should suffice in most cases. 2. can often be described by a model for the variation of the orbital elements, which is more akin to photodynamics.

@dfm
Copy link
Member Author

dfm commented Jan 17, 2020

@gjgilbert: Awesome!

I think that the best first step would be a mash up of SimpleTransitOrbit [src] with TTVOrbit [src] to make a SimpleVariationOrbit (?) that takes all the same arguments as the SimpleTransitOrbit plus ttvs, transit_times, transit_inds, and delta_log_period (see the TTVOrbit docstring for the definitions). Then we can generalize this to include transit_durations and transit_depths parameters. Looking at it quickly, it seems like the __init__ for SimpleVariationOrbit will be almost identical to TTVOrbit so perhaps we can break out the implementation into a separate superclass that knows how to handle TTVs? What do you think of that as a proposal?

If you're interested in putting this together, why don't you fork and open a pull request where we can start discussing the implementation in more detail. I've been messing with the git history a bit to clear out some cruft so you might want to delete your fork and make a new one if you have one already.

@gjgilbert
Copy link
Contributor

@dfm: Sounds like a plan! To clarify your proposal, you're suggesting that we also implement a superclass that contains both SimpleVariatonOrbit and TTVOrbit? I agree there will be quite a bit of overlap between the two.

I have a proposal deadline coming up soon, so I likely won't be able to start working on this in earnest for another week or two. I'll take a look over the source code and developer docs and circle back with you then.

@dfm
Copy link
Member Author

dfm commented Jan 17, 2020

Yes - I'm imagining that TTVOrbit would inherit from KeplerianOrbit and VariationOrbit (?), and that SimpleVariationOrbit (?) would inherit from SimpleTransitOrbit and VariationOrbit.

No rush! Let's stay in touch.

@gjgilbert
Copy link
Contributor

Hey @dfm, I'm picking this back up again. Quick thought we should discuss: do we want to work in terms of the total transit duration (1st to 4th contact) or the full transit duration (2nd to 3rd contact). I prefer 1st-4th, but if you have a strong argument otherwise, I'm open to being persuaded.

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

3 participants