Closed rodluger closed 4 years ago
Thanks! I'll look at this more tomorrow. But, the test failures are new as of your changes (and they're all related to the Keplerian orbit that you edited) so can take a look to see if you can track them down?
My bad -- there was a typo. Should be passing now.
For reference, here's my simple Python implementation of the iterative solver: https://gist.github.com/dfm/cfa2cbbae5b32e57536328564780e8f2
@dfm wow, your style guide is draconian! What should I run on my end to make the style builds pass?
Haha! It should just be black, isort, and black_nbconvert. No worries about getting it to pass. I'm happy to do it! Some day I'll write developer docs.
If you come back to this, make sure that you rebase (or just take a fresh clone) because I'm trying to clean up the history and get rid of the huge files...
OK will do.
I just re-visited this and I worked out the shape issues I think. Take a look and let me know what you think!
Also: @rodluger would you be able to add a few words to the docs about this? This could come in the form of a tutorial or just a longer docstring.
OK, looking into this now.
I added a really simple tutorial here. Sorry it's in *.ipynb
form, but I did strip the output.
Things look mostly OK, except that light_delay=True
doesn't play nice with use_in_transit=True
in get_lightcurve
. We'd have to change the contact points op to figure out the time of observed contact points. I could give that a shot, but I suspect it might be gnarly? Alternatively we could disable use_in_transit
when the user wants to apply the light delay. What kind of performance loss will that incur?
We could also just compute the maximum z-axis separation between the planet and the star and pad the transit window on the left by that amount to get a really conservative list of indices where transit might occur.
Great! I'd say we should just force use_in_transit
to be False
for now and revisit when the time comes.
I'm happy to merge this now that we're in the green if you're happy with it @rodluger. Thanks!
Light travel time delay computed by a second-order Taylor expansion of the retarded position. The expansion works well, and I created a test for it. The only issues I'm currently having are the shapes: for a single planet and/or a scalar evaluation time, my method to get the planet position returns a different shape than yours.
Also, the time delay is computed relative to the origin of the coordinate system, so transits happen early and eclipses happen late. This causes the observed transit time to be different than the
t0
provided by the user. One way around this is to define the light travel delay with respect to the point of conjunction with the observer. I can probably implement that but wanted to check with you first.