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

wip: implement a periodic tree that maps points to "mirrors" #193

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

KristofferC
Copy link
Owner

@KristofferC KristofferC commented Jun 20, 2024

Similar to what is described in https://namdanalyzer.readthedocs.io/en/latest/kdTree/periodic_kdtree.html.

then querying this kd-tree multiple times, if necessary, with all the relevant periodic images of the query point.

I don't understand why this is needed..

TODO:

  • Check query point is inside periodic box?
  • Docs
  • Tests

Fixes #133

@dkarrasch
Copy link
Contributor

I don't understand why this is needed..

I think this is because some points may be far from the query point, represented by some point in the canonical image, but close to a shifted one version of the query point. So you may need to query for the canonical version and the shifted one, and out of those find the nearest neighbors.

@KristofferC
Copy link
Owner Author

Aha, I misunderstood the link I think. What I thought they did was to duplicate the input points to all the mirrors (which is what this implementation does) but instead they map the input points to a single "canonical image". In the first case you only need to query once but in the latter you might need to query multiple times (for example if the ball around the query point is outside the image).

That is probably a better idea than what I did here.

@KristofferC KristofferC force-pushed the kc/periodic branch 2 times, most recently from 1f06239 to c57c643 Compare June 30, 2024 13:20
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.

periodic kdtree or balltree
2 participants