Open dfm opened 4 years ago
Thanks for considering this, @dfm.
I can think of two ways I'd want to interact with this feature as a user:
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.
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!
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!
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.
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.
@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.
@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.
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.
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.
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!