Solar system epherides doc update and kernel URL setting option - #4908

Actually, the bit about get_sun worries me. I think that for the planets as well we may prefer to give approximate positions (from erfa.plan94) since these are good enough for most purposes and do not require one to download the full ephemerides. If we do that, then there will be no need to make a special note for get_sun.

Is it silly to actually not download the kernel automatically on using get_body, but rather let the user do the download themselves via set_kernel? I think that if that kernel is set, get_sun should by default use it.

So, the idea would be that in the future we can still provide erfa.plan94 as a default (ideally still in the next few days...)

Sorry to be a bit late with this, but I actually do not like the idea that trying to get the position of the moon starts an automatic download of 120 MB...

@mhvk - I don't understand why you say:

If we do that, then there will be no need to make a special note for get_sun.

The point is that the ephemerides are better from DE430 than they are from get_sun or plan94 - but at the price of needing to download the kernel. So the note about get_sun is indicating "if you need a quick-and-dirty sun location, use get_sun, otherwise use get_body(t, 'sun'). But I think I just am not understanding what you mean there?

Is it silly to actually not download the kernel automatically on using get_body, but rather let the user do the download themselves via set_kernel? I think that if that kernel is set, get_sun should by default use it.

Hmm, I hadn't thought about that. But maybe it does make sense? Two things that require checking to decide for sure:

• How much of a "jump" is typical from get_sun(t) vs get_body(t, 'sun')? This might be substantial enough (i.e. the assumptions of get_sun are approximate enough) that it would be more confusing to have things suddenly change than it's worth.
• Does get_sun get used in some places that interact internally with other erfa code in coordinates? If so, that would have to be changed - some of the coordinates machinery (for precession and the like) depends much more on the consistency of the get_sun result with other things (i.e. the outputs of erfa.epv00) than it does on the actual value of get_sun. If need be it's a straightforward fix to change the current get_sun to something like _get_erfa_sun and then have get_sun be a shell over that and get_body... But that definitely needs checking.

I actually do not like the idea that trying to get the position of the moon starts an automatic download of 120 MB...

Oh, I agree. But frustratingly, that's the one thing that plan94 does not provide. More generally, there's no straightforward way at all to get the moon location from erfa. It might be that you can cobble together various convoluted things like combining the location of the Earth-Moon Barycenter with other quantities that deal with precession to infer it, but those are very hacky. Better would be to just find a paper with a decent polynomial fit to the moons orbital elements and use that... But no one has had the time/knowledge to do that yet. So I think it's better to have some way to do it, and perhaps later we can add a lower-precision-but-no-download version. (if that sounds reasonable, we should make an issue for it!)

I just checked and I think I am starting to understand plan94 well enough to make a quick PR that makes it the default for solar_system. My explicit suggestion would be to change the interface such that it is:

def get_body(planet, time, location=None, frame='gcrs', ephemeris='plan94'):
    ...

Here, I think initially:

1. location would be taken from time if not given; geocentric otherwise.
2. frame='icrs' would be a shortcut to get barycentric coordinates.
3. ephemeris=de* would lead one to set a kernel.

Before I proceed too much, does this make sense? @StuartLittlefair - what do you think?

Also, am not sure yet about get_moon using just erfa...

p.s. I'm still somewhat confused about why get_sun needs to calculate aberration. Would this be needed for the planet positions from plan94 as well?

Sorry, saw your comment only after committing: so, since it doesn't seem easy to get the Moon, I guess my being perhaps overly careful and unwilling to just download large files without thinking way, would suggests something an interface that currently is:

def get_moon(time, location=None, frame='gcrs', ephemeris=None):

which will raise an error unless an explicit ephemeris is passed in.

Your comment about the importance of get_sun being consistent with assumptions made in the coordinate transformations reminded me of another reason for wanting to proceed a little carefully: as is, @StuartLittlefair's implementation creates an ICRS position, which is at the ephemeris accuracy, but then uses the coordinate framework to get a position relative to Earth. It seems almost certain one can do somewhat better by using the ephemeris positions of the Earth itself. Indeed, in ultimate-accuracy coordinate transformations one probably wants to use the ephemeris. Obviously, it is fine to leave this for future work, though.

Another small comment: opening an SPK file is a remarkably fast operation (half the time of calculating one barycentric position), so it is not immediately obvious one has to keep the KERNEL alive (which makes a ephemeris='de430' option even easier to implement).

Will think a bit more & try things (but first off for a bit)

@eteq - a PR including plan94 is nearly done.... I'll post it and then we can discuss whether/how to merge

See #4920 for an attempt to include plan94. If you think this goes the right way, I'd be quite happy to combine the two PRs -- after we agree on API... Note that upon trying to implement, the frame option I suggested above seemed like it was the wrong approach, so I left that out, thus only adding an ephemeris argument (and swapping body and time).

I'll comment on the pros and cons of using plan94 as a kernel on @mhvk 's PR. Apart from my minor comment above I really like this PR.

Actually, @mhvk, do you mind if we merge this now (the tests should be fixed with the last commit) and then you rebase over that with the changes for #4920? I think that might be easier because this PR includes some cleanup of #4890 that's not strictly related to the changes you're making. (And that way we can move the rest of this discussion into one place in #4920

Yes, I can rebase on this -- I do like the science state way of doing things.

Alright, great. Once the tests pass I'll merge this (or you can feel free to do so yourself, @mhvk)

(the one travis failure is a red herring): merging!

FWIW, I was wrong about it being a red herring - I just mis-read the logs. But it's not fixed by 2e8fe46

Just a guess, the new error is "no newline at the end of file"? Then you need to add exactly one empty line at the end of the file.

Ignore this.