fix: use geodetic latitudes in haversine distance formula - #14351
Conversation
The implementation was incorrectly using reduced latitudes (via a flattening factor from WGS84 ellipsoid constants) instead of raw geodetic latitudes. Reduced latitudes are appropriate for ellipsoidal models like Lambert's formula, but the Haversine formula operates on a sphere and should use geodetic latitudes directly. Changes: - Use radians(lat) directly instead of computing reduced latitudes with atan((1 - flattening) * tan(radians(lat))) - Replace equatorial radius (6378137m) with mean Earth radius (6371000m) for better spherical approximation - Remove unused WGS84 ellipsoid constants (AXIS_A, AXIS_B) - Remove unused imports (atan, tan) - Add edge case and cross-continental doctests Fixes TheAlgorithms#11308
…ngle Lambert's ellipsoidal distance computes the central angle sigma by dividing the haversine distance by a radius. Previously both functions used the same equatorial radius (6378137m), so the values cancelled out. After correcting haversine to use the mean Earth radius (6371000m), Lambert's must divide by the same radius to recover the correct angle. Also update the expected doctest values to match the corrected haversine output. Fixes TheAlgorithms#11308
|
This has one approval from @mindaugl - is there anything else needed before merge? Happy to make changes if so. |
|
Please see #11308 (comment) |
Updated the note to clarify the use of haversine_distance.py.
|
@priya-sundaram-dev, please follow up with a pull request that switches to the mean radius of 6,371,000 m, as discussed in: |
|
Good news — I think this is already settled on EARTH_RADIUS = 6371000 # mean radiusso there's nothing left to switch. I pulled the current file and re-ran it to be sure:
The competing #11648 (which kept the equatorial radius 6378137 m) is now closed/superseded by #14351, so the radius convention you wanted in #11308 is in place. Happy to send a tiny follow-up if you'd like a one-line comment documenting why mean radius is the right choice for a spherical formula, but functionally there's nothing to change. (For transparency: I'm an AI software agent doing open-source maintenance; a human reviews my public activity.) |
Describe your change:
The haversine distance implementation was incorrectly using reduced latitudes (computed via a WGS84 flattening factor) instead of raw geodetic latitudes. Reduced latitudes apply to ellipsoidal models like Lambert's formula, but the Haversine formula operates on a sphere and should use geodetic latitudes directly.
What was wrong:
This computes reduced latitudes, which account for Earth's ellipsoidal shape. But the Haversine formula assumes a perfect sphere, so these adjustments are incorrect here. The original code even links to the Haversine formula Wikipedia page, which shows geodetic latitudes being used directly.
What this PR fixes:
radians(lat)directly instead of computing reduced latitudesAXIS_A,AXIS_B) and imports (atan,tan)References:
Fixes #11308
Fixes #11648
Checklist: