-
Notifications
You must be signed in to change notification settings - Fork 35
Open
Labels
Description
Problem
In the current design of jplephem's SPK interface, the compute and compute_and_differentiate methods behave inconsistently between type 2 and type 3 segments. This inconsistency leads to confusion and actual bugs in user code, such as in astropy's solar system ephemeris calculations.
Details
- For type 2 segments, both methods work as expected—returning 3D position and 3D velocity, respectively, with well-documented units.
- For type 3 segments,
computereturns 6 components:[x, y, z, dx, dy, dz], where dx, dy, dz are velocity components. However, the velocity units are always in km/s, contrary to the user's assumption (often expecting km/day, as with type 2). - Moreover, the
compute_and_differentiatemethod returns the 6D components and their derivatives, but only the first 6D output is the correct position and velocity; the derivatives output is not suitable for physical velocity interpretation.
Real Bug Example in Astropy
In astropy/astropy/files/astropy/coordinates/solar_system.py, in the function:
def _get_body_barycentric_posvel(body, time, ephemeris=None, get_velocity=True)the code for type 3 segments is:
if spk.data_type == 3:
# Type 3 kernels contain both position and velocity.
posvel = spk.compute(jd1, jd2)
if get_velocity:
body_posvel_bary += posvel.reshape(body_posvel_bary.shape)
else:
body_posvel_bary[0] += posvel[:3]Here, the velocity (posvel[3:6]) is incorrectly assumed to be in km/day, resulting in erroneous output because the actual unit is km/s.
Why This Is Harmful
- The API exposes the internal type-specific format in the external interface, requiring users to understand SPK internals to correctly interpret output.
- Downstream consumers (like astropy) create bugs from incorrect unit assumptions.
- Documentation does not call out this subtlety loudly enough or offer unified, type-independent results.
Suggested Solution
- Ensure
computealways returns just the position vector (3D), with consistent units, for both type 2 and type 3 segments. - Ensure
compute_and_differentiatealways returns a(position, velocity)tuple, with velocity units explicitly documented (preferably standardized, e.g. always km/s or km/day, with correct conversion for type 3). - Update documentation to highlight this change and warn about prior inconsistencies for downstream libraries.
- Consider providing automated unit conversion internally so users do not have to worry about SPK kernel types or units.
Impacted Users
- Any code that relies on jplephem/SPK, including major astronomy projects like astropy, is affected.
- Example code in astropy directly demonstrates the real-world impact and propagation of the bug.
Reactions are currently unavailable