add default value of Earth's radius to Haversine #176
Conversation
|
An additional concern could be that the function assumes the first point to be longitude, the second latitude, but that's not documented anywhere. Add to readme? |
I think it's mentioned in the docstring? But adding a short remark to the README may be helpful as well. Also, the default value including its unit should be stated in the docstring. |
src/haversine.jl
Outdated
| @@ -9,6 +9,7 @@ The computed distance has the same units as that of the radius. | |||
| struct Haversine{T<:Real} <: Metric | |||
| radius::T | |||
| end | |||
| Haversine() = Haversine(6378137.0) | |||
There was a problem hiding this comment.
Maybe it would be better to make this a Float32 or an Int32 to avoid promoting Float32 values to Float64 unnecessarily?
|
Good idea. Though most references I found use 6371 km, which is the mean radius and recommended according to Rosetta code: References that mention 6378 km often also mention 6371 km and indicate it depends on where you are on earth. So it seems 6371 km may be more standard? See also mapbox/turf-swift#26 and linked issues for a discussion. |
Codecov Report
@@ Coverage Diff @@
## master #176 +/- ##
==========================================
- Coverage 97.99% 97.86% -0.14%
==========================================
Files 8 8
Lines 748 749 +1
==========================================
Hits 733 733
- Misses 15 16 +1
Continue to review full report at Codecov.
|
|
I didn't look too closely, but I agree that, in my area, people seem to use 6371 km, which, according to the Earth Fact Sheet linked in the OP, corresponds to the volumetric mean radius. |
I don't think it should be necessary to look up the radius of Earth every time one wants to use the Haversine formula, so I've added the equatorial radius of Earth in meters as a default. I hope most imperial unit users will agree that metric is a reasonable default for scientific libraries. Of course Earth has different radii depending on where it is measured, as it isn't a perfect sphere. Uses sensitive to this should use more specific values. FYI this same default value is used in R. source https://nssdc.gsfc.nasa.gov/planetary/factsheet/earthfact.html
|
Sorry for delay - I've edited as requested |
|
Sorry for not commenting earlier, but this needs a test. |
|
I have tests in #203. |
I don't think it should be necessary to look up the radius of Earth every time one wants to use the Haversine formula, so I've added the equatorial radius of Earth in meters as a default. I hope most imperial unit users will agree that metric is a reasonable default for scientific libraries. Of course Earth has different radii depending on where it is measured, as it isn't a perfect sphere. Uses sensitive to this should use more specific values. FYI this same default value is used in R. source https://nssdc.gsfc.nasa.gov/planetary/factsheet/earthfact.html