Repository navigation
Use new erfa routine to calculate the position of the Moon - #11753
Conversation
|
cc also @mkbrewer who can perhaps shine some light on why the positions match relatively poorly. |
mhvk
changed the title
|
@mhvk Can you please post the error in RA/Dec that you see compared to JPL Horizons. Error in both ICRS and TETE would be helpful. |
|
Actually, just the date, JPL ICRS and TETE positions along with the corresponding Astropy positions would be good. |
|
Of course, you could also just use the Astropy output for DE430 in place of JPL Horizons. |
|
@mkbrewer - I think I part of the problem may be that I misunderstood the statement about the accuracy; the documentation lists:
I interpreted these as RMS errors in the norm of the position differences, but now that I check more carefully (in GRCS), I see that I get 3" rms for the difference in RA and 1.5" in dec, so those are OK. The position I'm still confused about. The rms in distance difference I get is 27 km, and in X, Y, and Z, it is 21, 18, and 6 km. I now wonder if it is just the documentation not being quite right... I did some checks with the code below. But since the numbers are essentially the same with the old code we had, I think for the purposes of this PR it is probably OK. which produces |
1a9745c to
80a5647
Compare
| body_pos_bary = icrs_cart.without_differentials() | ||
| body_vel_bary = icrs_cart.differentials['s'].to_cartesian() | ||
| # TODO: the Meeus algorithm includes light travel time. | ||
| # That should be undo somewhere |
There was a problem hiding this comment.
I note that, in fact, light travel time is omitted:
** 2) This function is a full implementation of the algorithm
** published by Meeus (see reference) except that the light-time
** correction to the Moon's mean longitude has been omitted.
However, given the stated accuracy and the fact that the effect of light travel time to the geocenter amounts to only 0.7" in longitude, it is likely OK to omit the calculation of the light delayed position.
There was a problem hiding this comment.
Thanks for looking much more carefully! It actually is nicer - I think we might as well include the light travel time correction, making the case of the moon less special.
Transversal position errors of 6 km are matching 3" at the distance of the Moon well enough; maybe would need a look into the Meeus paper to find out if a larger uncertainty in distance is to be expected. Also, it's not fully outside the given range or errors
OTOH 27 km feels uncomfortably close to the Earth's orbital motion over one light travel time, so maybe that effect is still missing somewhere. Are you getting comparable differences at different phases? |
|
I looked into that and he is not. The fact that he is currently using the calculated GCRS position directly rather than differencing the BCRS positions of the Moon and the Earth precludes any effect of the orbital velocity of the Moon around the SSB. |
|
@mkbrewer, @dhomeier - I've started a new issue, #11755, since I think one thing is clear is that this is not related to moving to a new implementation - the answers between our own implementation of Meeus and |
5f0c247 to
14a56fd
Compare
|
@pllim -as our "actions" expert, for this PR I've been getting messages that all the action tests are cancelled as duplicates, even though I'm pushing up new stuff. Could it be that somehow things are being run both on my branch as on astropy? As far as I know, though, I have not changed any setting... |
|
GitHub Actions is having problems right now: Update - We continue to observe our services recover with an increasing number of successful builds, we will keep monitoring until full recovery and will provide more updates soon. May 18, 16:08 UTC The logs say, "GitHub Actions has encountered an internal error when running your job," which is caused by the upstream problem, not our cancel job. |
|
Thanks! |
|
I've converted to draft while some additional thinking is going on. My latest addition makes distance better but RA, Dec worse... |
14a56fd to
658a6dd
Compare
|
OK, instead of making changes to the light travel time corrections, I now again left it as it was, and just added comments with links to #11755. For the purposes of this PR, which is just to switch out our own implementation of the Meeus algorithm with the SOFA one, I think that is fine. The two codes give consistent results. So, ready for review, I guess... |
|
@mhvk now that @mkbrewer has diagnosed the issues relating to light travel time in #11755 (comment), do you want to implement that change in this PR, or shall I review this PR as-is and you can submit a new PR? I slightly favour the latter. Then this PR essentially keeps functionality the same, just using |
|
@StuartLittlefair - I actually am tempted to just do it right in one go. A consequence of the realization that |
Since moon98 returns the astrometric position relative to Earth, this meant that the best place to use it is inside _get_body_barycentric_posvel, and that then no further special casing is necessary.
658a6dd to
dff3564
Compare
|
@StuartLittlefair, @mkbrewer - OK, the PR is now revised. I like that the code is now much simpler, and of course it helps that it is more precise too! |
|
@mhvk This looks good to me. |
|
OK, great. And thanks of course very much for figuring out what the problem was! And showing that we had not done it quite right until now... |
This replaces our own implementation of the Meeus algorithm for calculating the position of the Moon with the new routine in
erfa(so the PR also bumps the minimum version of erfa).I added a comparison between the JPL ephemeris and this new one, and
oddly the differences in position and velocity are quite a bit larger than they should be according to thewith the correct implementation pointed out by @mkbrewer the code is now consistent in precision with what was expected.moon98documentation (see https://pyerfa.readthedocs.io/en/latest/api/erfa.moon98.html#erfa.moon98).EDIT: old text for reference only
In the PR, I write we should also be sure to correct for light-travel time effect (which is included in the Meeus algorithm, but should not be for barycentric position), but from the trials I did I'm not sure that is enough. Am I missing something? (I tried applying in GCRS, which has almost no effect; I also tried applying the Moon-Earth light travel time one finds from the GCRS to the ICRS pos and vel, which does give an improvement but not to the level expected).Fixes #11739, #11755