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

Add SearchTree Class and integrate with VoronoiMeshSnapshot #226

Closed
wants to merge 1 commit into from

Conversation

arlauwer
Copy link
Contributor

@arlauwer arlauwer commented Dec 6, 2024

Description
Introduced a new SearchTree class in the utils/ directory, implemented as a general-purpose k-d tree. The class provides a spatial data structure for efficient nearest-neighbor searches to an input position in O(log(N)) time.

Additionally, the VoronoiMeshSnapshot class has been updated to replace its private nested Node class with the new SearchTree.

Motivation
The upcoming Tetrahedral grid requires a k-d search tree. Moving the search tree implementation to a shared class improves code maintainability. This class is now also available for future features requiring k-d tree functionality.

Tests
All tests passed, including all Voronoi tests that rely on the new SearchTree. No additional tests were deemed necessary, as the current tests sufficiently cover the implementation of the SearchTree.

Code Guidelines
Efforts were made to adhere to SKIRT guidelines. Feel free to highlight any required corrections.

Additional changes to VoronoiMeshSnapshot

  • The Cell class no longer stores the site positions. Instead, these are maintained in a separate field: std::vector<Vec> _sites. This change was necessary to allow for the generic implementation of SearchTree, which operates on a std::vector of node positions. The _blocktrees and _blocklists remain within the VoronoiMeshSnapshot class, as these are only relevant when the Voronoi cell or its bounding box are available for every node. Since a node belongs to a block precisely if its Voronoi cell (or bounding box for simplicity) intersects that block.
  • A new removeCells method ensures that _cells and _sites remain synchronized when cells are removed.
  • The buildMesh function previously used std::sort for efficiently finding cells that were too close. This sort now has to be applied to both _cells and _sites, ensuring consistent ordering between the two. The current approach may seem somewhat convoluted, but it remains efficient.

Adjusted VoronoiMeshSnapshot to use the new SearchTree
@petercamps
Copy link
Contributor

After discussion with @arlauwer we decided to abandon this pull request because:

  • as it is proposed here, the interface between the search tree class and its client makes things fairly complicated;
  • the nearest neighbor search is not needed for the upcoming tetrahedral grid after all.

We thus believe it is best to keep the current implementation, with the search tree embedded in the Voronoi snapshot class.

@petercamps petercamps closed this Dec 10, 2024
@arlauwer arlauwer deleted the searchtree branch December 11, 2024 10:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants