search

AliceAdenis / maps

1 stars 1 forks source link

great circle distance #1

Open krishaizl opened 5 years ago

krishaizl commented 5 years ago

Hello Alice, great work with your modified RBF kernel and awesome articel on medium.com. Helped me a lot to get into the gaussian regression as i need it.

But i just stumbled over an feature/issue in your great_circle_distance function. To have correct distances of the output one needs to multiplikate it with earth's radius ... otherwise it will just be radiantpart.

is it supposed to be like that? if it is, maybe a info in the comment of the function would be nice.

best regards, kris

AliceAdenis commented 5 years ago

Hi Kris, Your right, the Haversine formula actually mention a multiplication by the radius that I did not made. I will add a parameter 'r' for the radius, and some docstring to the function. Thanks a lot, :) Alice

naught101 commented 2 years ago

@AliceAdenis Have you considered submitting this as an PR to scikit-learn? There doesn't seem like an obvious reason why a metric choice wouldn't make sense in the default RBF.

Also, you could make a simpler wrapper function around the haversine function that sets the radius, and just pass that in, to save having to modify the RBF code. That would work better if this is something that could be merged into scikit-learn.