Add support for heliocentric coordinate frames and barycentric/heliocentric time corrections #4314

StuartLittlefair · 2015-11-16T19:14:57Z

This pull request fixes #2244, fixes #4288, and adds tests for the GCRS<->ICRS transformations in #4268. The idea is that helper routines, similar to sidereal_time are now attached to each Time object. These helper routines, heliocentric_correction and barycentric_correction return TimeDelta objects, which can be added to a Time object to create a heliocentric or barycentric corrected time.

An example usage

from astropy import coordinates as coord, units as u
from astropy.time import Time

wht  = coord.EarthLocation.of_site('lap alma')
star = coord.SkyCoord("08:08:08 +32:00:00",distance=120*u.pc,unit=(u.hour,u.degree),frame='icrs')
t = Time("2013-02-02T23:00")
t.location = wht
hcor_val = t.heliocentric_correction(star)
bcor_val = t.barycentric_correction(star)

# the returned timedelta objects can be added to the original time
hcor = t + hcor_val
bcor = t + bcor_val
print(' MJD (UTC)  = %.11f' % t.mjd)
print(' MJD (TT)   = %.11f' % t.tt.mjd)
print(' MJD (TDB)  = %.11f' % t.tdb.mjd)
print('HMJD (UTC)  = %.11f' % hcor.utc.mjd)
print('HMJD (TT)   = %.11f' % hcor.tt.mjd)
print('HMJD (TDB)  = %.11f' % hcor.tdb.mjd)
print('BMJD (UTC)  = %.11f' % bcor.utc.mjd)
print('BMJD (TT)   = %.11f' % bcor.tt.mjd)
print('BMJD (TDB)  = %.11f' % bcor.tdb.mjd)
print("Correction to add to get time at heliocentre = {} seconds".format(hcor_val))
print("Correction to add to get time at barycentre = {} seconds".format(bcor_val))

eteq · 2015-11-17T13:40:01Z

@StuartLittlefair - before doing a detailed review, there are some conceptual bits I'm confused by here. In the docstrings you call Heliocentric "relative to the Earth's center-of-mass rather than the solar system Barycenter". Why is that "Heliocentric"? (Or is that maybe a typo?) Similarly, in #4314 you mention the need for a barycentric coordinate system, but that's not in here. Am I right in interpreting this code that you've decided that ICRS is right for those purposes?

StuartLittlefair · 2015-11-17T13:51:04Z

@eteq Thanks for the questions.

Yes - I meant relative to the Sun's centre of mass in the docstrings. I'll make that change.
w.r.t to Add support for heliocentric coordinate frames and barycentric/heliocentric time corrections #4314 I did indeed realise that ICRS is fine for this purpose since it is a Barycentric coordinate system. Moreover, since it's the coordinate system in which the star's vector is most commonly given, it is ideal for calculating light travel times

mhvk · 2015-11-17T19:30:28Z

astropy/coordinates/builtin_frames/icrs_cirs_transforms.py

Putting in comments as I look over things, some of which may be quite nitpicky: Getting the unit here by converting to cartesian seems extremely expensive. I think this would do

not all(c.unit.is_equivalent((u.one, u.radian)) for c in grcs_coo.data.components)

The code you suggest doesn't work - gcrs_coo.data.components gives a tuple of strings - (lat, lon) for example. I don't think it's that expensive either, since the cartesian representation is only calculated if the first condition is not true, which I think it always should be.

mhvk · 2015-11-17T20:05:07Z

@StuartLittlefair - This looks great! As you'll have seen, I have only very minor comments, except that I think the code for the two corrections in Time can be combined (either using a private method or by defining a method that allows the user to choose between barycentric and heliocentric; I think I prefer the latter, with the option set by strings rather than the coordinate class; this has an analogy in how sidereal_time is implemented, with its options for kind).

mhvk · 2015-11-17T20:08:16Z

One important aspect will be to document it well. In particular, to get barycentric times, one would do,

t_barycentre = time.tdb + time.barycentric_correction(coord)

By returning a TimeDelta on the TDB scale, you already ensure any errors from not doing .tdb as above (but perhaps after the addition) are minimized, but it is probably good if the docstring and documentation make abundantly clear that in the barycentre one should use tdb.

mhvk · 2015-11-17T20:09:50Z

astropy/coordinates/builtin_frames/icrs_cirs_transforms.py

This one is covered by #4302 -> needs a rebase.

mhvk · 2015-11-17T20:11:57Z

Ah, yes, another one worth bringing outside of the in-line comments is that you currently assume the distance to the object is far larger than the distance between the observatory and the barycentre. In principle, this is not necessary (though in practice one has to do the trigonometry carefully to ensure one does well in the small-angle limit). Not sure it is worth addressing here...

mhvk · 2015-11-17T20:37:54Z

astropy/time/core.py

I think the dot only words for scalar tcor and spos; should it be (spos * hpos).sum(axis=-1) / const.c? (This leaves properly broadcasting up to the user.)

playing around with this it looks like this should be (spos*cpos).sum(axis=0) / const.c?

StuartLittlefair · 2015-11-18T11:11:19Z

I've pushed a lot of changes to address @mhvk's comments.

I've also attempted to rebase to bring this in line with #4302 following the instructions in the astropy docs. This is new to me and exceeds my git comfort levels, so I apologise if i stuffed it up.

mhvk · 2015-11-18T14:19:46Z

astropy/coordinates/builtin_frames/heliocentric.py

You're not using EQUINOX_J2000 (also missing end of line at end of file).

…e heliocentre

…x vector times

…le obstimes

…ts for same

mhvk · 2016-03-23T13:24:53Z

OK, tests now passed, so merging. Thanks very much, @StuartLittlefair, for fixing this very longstanding issue of not being able to calculate barycentric corrections!

eteq · 2016-03-23T16:29:33Z

Oops, missed that there was some recent progress here not related to climbing... But it looks great, thanks @StuartLittlefair and @mhvk!

embray added coordinates Affects-release Enhancement labels Nov 16, 2015

mhvk reviewed Nov 17, 2015
View reviewed changes

astropy/coordinates/builtin_frames/icrs_cirs_transforms.py Outdated

Copy link

Contributor

mhvk Nov 17, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This one is covered by #4302 -> needs a rebase.

mhvk reviewed Nov 17, 2015
View reviewed changes

mhvk reviewed Nov 18, 2015
View reviewed changes

StuartLittlefair and others added 25 commits March 23, 2016 07:44

syntax error fix

11f376e

docstring fixes of typos and incorrect magnitude for aberration at th…

376c110

…e heliocentre

document origin of tests and newline eof

06db62e

moved to single ltt_correction function, include documentation and fi…

9aab43c

…x vector times

removed use of transpose to deal with shaped coordinates

427165b

used new get_jd12 helper function

d49f4b2

changes to docstring example

e01df36

end of line at end of file and delete unused imports

46ba8fb

np.rollaxis changes to work with more complex shapes

f3fb04c

PEP8, cleanup, bug fixes.

93a4806

changed ltt_correction to light_travel_time

3474ea5

removed example from docstring of light_travel_time function

6064bbe

added example to Time documentation

2d3b95c

typo in docstring

ac1e053

changes to cartrepr_from_matmul to handle single locations and multip…

7bc827d

…le obstimes

changed coordinate system name from Heliocentric to HCRS

6e69d38

fixing bug introduced in 984561c

f09b245

renamed test class and added array test

796ec04

switch to duck-typing for location and skycoord objects

984dde0

removing test_heliocentric function left after rebase

afa0c72

adding GCRS frame to import

3be0b66

name change to test class

8b031a6

changing location of test observatory so tests pass

87c3489

tweaks to heliocentric corrections to deal with vector times, and tes…

7b59f13

…ts for same

PEP8 corrections

58effd0

mhvk merged commit 4f86d0e into astropy:master Mar 23, 2016

mhvk mentioned this pull request Mar 29, 2016

Solar system and lunar ephemerides via jplephem #4746

Closed

StuartLittlefair mentioned this pull request Jun 14, 2016

Constraint broadcasting astropy/astroplan#165

Merged

Uh oh!

Add support for heliocentric coordinate frames and barycentric/heliocentric time corrections #4314

Add support for heliocentric coordinate frames and barycentric/heliocentric time corrections #4314

Uh oh!

Conversation

StuartLittlefair commented Nov 16, 2015

Uh oh!

eteq commented Nov 17, 2015

Uh oh!

StuartLittlefair commented Nov 17, 2015

Uh oh!

mhvk Nov 17, 2015

Choose a reason for hiding this comment

Uh oh!

StuartLittlefair Nov 18, 2015

Choose a reason for hiding this comment

Uh oh!

mhvk commented Nov 17, 2015

Uh oh!

mhvk commented Nov 17, 2015

Uh oh!

mhvk Nov 17, 2015

Choose a reason for hiding this comment

Uh oh!

mhvk commented Nov 17, 2015

Uh oh!

mhvk Nov 17, 2015

Choose a reason for hiding this comment

Uh oh!

StuartLittlefair Nov 18, 2015

Choose a reason for hiding this comment

Uh oh!

StuartLittlefair commented Nov 18, 2015

Uh oh!

mhvk Nov 18, 2015

Choose a reason for hiding this comment

Uh oh!

mhvk commented Mar 23, 2016

Uh oh!

eteq commented Mar 23, 2016

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

9 participants