search

mapado / haversine

Calculate the distance between 2 points on Earth
MIT License
326 stars 62 forks source link

Possible vectorization bug #72

Closed ol-freelance closed 8 months ago

ol-freelance commented 8 months ago

In haversine 2.8.0 at lines 173-174 you have:

 if has_numpy:
        _haversine_kernel_vector = numba.vectorize(fastmath=True)(_haversine_kernel)

Shouldn't the second line read instead

        _haversine_kernel_vector = numba.vectorize(fastmath=True)(_haversine_kernel_vector)
jdeniau commented 8 months ago

Reading this, it may be. Do you have a reproducible issue that we might add it a unit test case?

ol-freelance commented 8 months ago

It doesn't change the result, only performance, so I don't see how you could test-case it (performance tests are not unit tests)

It's more a question of intent: in that branch you first test for has_numpy and then you go and use the non-numpy kernel. So something is not right there: either numba for some reason can't take a numpy kernel and then there is no point testing for numpy and conversely if it can when why not use the numpy kernel?

jdeniau commented 8 months ago

Fixed in 2.8.1