Repository navigation
Fixed missing induced velocity for helioecliptic transforms - #10970
Conversation
|
Hello. Is this a bug fix or enhancement? Thanks! |
|
Bug fix: the velocity transformations between ICRS and any of the helioecliptic frames are currently not accurate (by a shift of the velocity of the Sun: ~15 m/s) |
mhvk
left a comment
mhvk
left a comment
There was a problem hiding this comment.
This looks very good! Since it is a bug fix, I think we should backport to 4.0.4. Could you add a changelog entry?
I don't really have any substantial comments, except that I'm not very keen on the names! But admittedly I don't really have good alternative suggestions either.
| return earth_pv, earth_heliocentric | ||
|
|
||
|
|
||
| def offset_sun_from_barycenter(time, include_velocity=False, reverse=False): |
There was a problem hiding this comment.
It is very annoying that representations cannot be simply negated, as that would avoid reverse here. In principle, that is not difficult to solve, but would be a new feature, so not so helpful here (but see #10987). Looking at solar_system, I also think that, really, get_body_barycentric itself should have the include_velocity option. We could consider calling it get_sun_barycentric(time, include_velocity=False, reverse=False) here (and then later deprecate reverse).
|
mhvk
left a comment
mhvk
left a comment
There was a problem hiding this comment.
Looks good modulo the typo noted - can just be a [skip ci] commit.
|
Whoops! Typo fixed |
mhvk
left a comment
mhvk
left a comment
There was a problem hiding this comment.
OK, looks all good now! Merging...
Fixed missing induced velocity for helioecliptic transforms
Fixed missing induced velocity for helioecliptic transforms
Fixed missing induced velocity for helioecliptic transforms
Fixed missing induced velocity for helioecliptic transforms
Fixed missing induced velocity for helioecliptic transforms
Fixed missing induced velocity for helioecliptic transforms
Fixed missing induced velocity for helioecliptic transforms
Fixed missing induced velocity for helioecliptic transforms
Fixed missing induced velocity for helioecliptic transforms
3 participants
The affine transformations between ICRS and any of the helioecliptic frames (
HeliocentricMeanEcliptic,HeliocentricTrueEcliptic, andHeliocentricEclipticIAU76) are missing the induced-velocity offset if the to-be-transformed coordinate has differentials. I move the code from the ICRS<->HCRS transformations to a separate function inastropy.coordinates.utilsand used it for all of the transformations.