Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

inconsistent return value of find_k_nearest_gpi #75

Closed
sebhahn opened this issue May 24, 2022 · 8 comments
Closed

inconsistent return value of find_k_nearest_gpi #75

sebhahn opened this issue May 24, 2022 · 8 comments

Comments

@sebhahn
Copy link
Member

sebhahn commented May 24, 2022

Depending on whether the NN search returns a distance containing a np.inf or not decides whether the returned indices and distances are a 1d array (mask applied) or a 2d array (no mask applied, i.e. no np.inf entry in the distances)

https://github.com/TUW-GEO/pygeogrids/blob/master/src/pygeogrids/grids.py#L468

@wpreimes
Copy link
Member

wpreimes commented May 24, 2022

should we make it consistent?

sebhahn added a commit that referenced this issue May 24, 2022
@sebhahn
Copy link
Member Author

sebhahn commented May 24, 2022

yes I think so, however, I'm not sure if a 2d array is the actual intention

according to the tests it might be, I'm just going through them

@wpreimes
Copy link
Member

I think I changed that part of the code some time ago, and I made sure that it works the same way as before. But I also was not sure at that time if it was on purpose or not. I think it can always be 1d.

@sebhahn
Copy link
Member Author

sebhahn commented May 24, 2022

no, I think it has to be 2d because it is possible to pass multiple lat/lon coordinates and get k neighbors in return.
using a boolean mask reduces the 2d array to a 1d array, so one solution could be keeping also distances with np.inf otherwise the number of neighbors might be different querying two coordinates at the same time

@wpreimes
Copy link
Member

ah ok, makes sense

@sebhahn
Copy link
Member Author

sebhahn commented May 24, 2022

I hope changing this behavior is not breaking any code

@sebhahn
Copy link
Member Author

sebhahn commented May 24, 2022

@wpreimes how should we handle np.inf in case of find_nearest_gpi?

a) return np.inf
b) or return empty array/None?

s-scherrer added a commit that referenced this issue Jan 18, 2023
@wpreimes
Copy link
Member

fixed in #76

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants