Skip to content

fix: modify KNN to also explore boundaries that do not contain the query point#1

Open
JWSong wants to merge 6 commits intomainfrom
fix/let-knn-contains-different-boundaries
Open

fix: modify KNN to also explore boundaries that do not contain the query point#1
JWSong wants to merge 6 commits intomainfrom
fix/let-knn-contains-different-boundaries

Conversation

@JWSong
Copy link
Copy Markdown
Collaborator

@JWSong JWSong commented May 18, 2024

This will resolve the issue where the K-Nearest Neighbors algorithm cannot find points that are outside of the boundary to which it belongs.

@JWSong JWSong requested review from Eveheeero and Migorithm May 18, 2024 13:35
@JWSong JWSong self-assigned this May 18, 2024
Comment thread src/lib.rs
pub latitude: NonNanFloat,
}

pub trait TCoordianteFloat {
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

conversion logic is already implemented for f64. Perhaps, Trait is not necessary?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just wanted to limit the types it can take.

Comment thread src/lib.rs
self.search(&mut nearest_neighbors, query_point, k);

nearest_neighbors
.into_sorted_vec()
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what would the cost of into_sorted_vec like?

rust-lang/rust#115357

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Considering the use case, it seems unlikely that we will need to use a large value for 'k'.

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

Successfully merging this pull request may close these issues.

2 participants