Skip to content

Conversation

@matthew-mccall
Copy link
Contributor

This pull request refactors and generalizes the point search functionality in the codebase, with a focus on updating the 2D search to use new class and type names, improving the search result logic, and adding support for higher dimensions. The changes also introduce new functors and intersection map constructors for both 2D and 3D cases, and update the interfaces and result handling to use more descriptive and robust structures.

Refactoring and Generalization of Point Search:

  • Replaced GridPointSearch and related types with GridPointSearch2D and associated result types throughout the codebase, updating construction and usage to use std::unique_ptr and new class names (OmegaHField, adj_search.hpp, point_search.cpp). [1] [2] [3] [4] [5]
  • Updated the result logic in the point search operator to distinguish between vertex, edge, and face hits, using tolerances for proximity checks, and storing the nearest element ID and parametric coordinates. The result struct and dimensionality enum were updated for clarity. [1] [2]

Support for Higher Dimensions:

  • Added generic bounding box and intersection routines (simplex_bbox, within_bbox, simplex_intersects_bbox, bbox_verts_within_simplex) templated by dimension, enabling use for both 2D and 3D meshes. [1] [2] [3] [4]

New Functors and Intersection Map Constructors:

  • Introduced GridTriIntersectionFunctor2D and GridTriIntersectionFunctor3D for mesh/grid intersection logic, with corresponding construct_intersection_map_2d and construct_intersection_map_3d functions for building candidate maps in 2D and 3D. [1] [2]

Interface and Type Updates:

  • Updated interfaces and result handling in search-related classes and functions to use new types and enums, and replaced deprecated or custom barycentric calculations with calls to Omega_h::barycentric_from_global. [1] [2]

These changes modernize the point search infrastructure, enable future extension to higher dimensions, and improve the clarity and reliability of search results.

@jacobmerson
Copy link
Collaborator

Note: this was moved here from jacobmerson#11 . Some preliminary discussion happened there.

@jacobmerson
Copy link
Collaborator

@matthew-mccall at least the initial CI test failures should be resolved from this PR: #216

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This pull request refactors the point search functionality to support both 2D and 3D meshes, introducing a class hierarchy with PointLocalizationSearch as a base class and dimension-specific implementations (GridPointSearch2D and GridPointSearch3D). The changes add tolerance-based proximity checking for vertices and edges in 2D, generalize bounding box routines to work with arbitrary dimensions, and lay the groundwork for 3D point localization.

  • Refactored GridPointSearch to GridPointSearch2D with a new base class PointLocalizationSearch<dim>
  • Added tolerance-based searching for vertices and edges in 2D with configurable proximity thresholds
  • Introduced GridPointSearch3D with basic region containment checking (without tolerance support yet)

Reviewed Changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 7 comments.

Show a summary per file
File Description
test/test_point_search.cpp Updated tests to use new GridPointSearch2D API, added tolerance configuration, and updated test expectations for vertex/edge/face classification
src/pcms/uniform_grid.h Removed default template parameter from UniformGrid and added explicit Uniform3DGrid typedef
src/pcms/point_search.h Introduced PointLocalizationSearch<dim> base class, split into GridPointSearch2D and GridPointSearch3D derived classes with tolerance support
src/pcms/point_search.cpp Refactored 2D search with tolerance-based vertex/edge detection, added dimension-agnostic helper functions (simplex_bbox, within_bbox, bbox_verts_within_simplex), and implemented basic 3D search
src/pcms/adapter/omega_h/omega_h_field.h Changed from std::optional<GridPointSearch> to std::unique_ptr<PointLocalizationSearch2D> for polymorphism
src/pcms/interpolator/adj_search.hpp Updated to use GridPointSearch2D and renamed tri_id to element_id
Comments suppressed due to low confidence (1)

src/pcms/point_search.cpp:660

  • In 3D, Omega_h::FACE refers to triangular faces of tetrahedra, not the tetrahedral elements themselves. For 3D point search, you likely want connectivity from regions (tetrahedra) to faces/edges/vertices. The current code queries face-to-edge and face-to-vertex adjacency, which may not be the intended behavior for locating points within tetrahedral elements. Consider using Omega_h::REGION instead of Omega_h::FACE for the 3D case.
  tris2edges_adj_ = mesh.ask_down(Omega_h::FACE, Omega_h::EDGE);
  tris2verts_adj_ = mesh.ask_down(Omega_h::FACE, Omega_h::VERT);
  edges2verts_adj_ = mesh.ask_down(Omega_h::EDGE, Omega_h::VERT);

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

bool edge_found = false;
bool inside_cell = false;

auto nearest_element_id = candidates_begin;
Copy link

Copilot AI Nov 16, 2025

Choose a reason for hiding this comment

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

The variable nearest_element_id is initialized to candidates_begin, which is an index into the candidate map, not an actual element ID. If no elements are found within tolerance or inside the mesh, this will result in returning an incorrect element ID. Consider initializing to -1 or 0 to indicate "not found", and only update it when a valid element is identified.

Suggested change
auto nearest_element_id = candidates_begin;
auto nearest_element_id = -1;

Copilot uses AI. Check for mistakes.
auto candidates_end = candidate_map.row_map(cell_id + 1);
bool found = false;

auto nearest_triangle = candidates_begin;
Copy link

Copilot AI Nov 16, 2025

Choose a reason for hiding this comment

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

Same issue as line 435 in 2D: nearest_triangle is initialized to candidates_begin, which is an index into the row map, not an actual element ID. If no tetrahedron contains the point, this will return an incorrect negative element ID derived from the wrong value. Initialize to -1 or ensure it's set to a valid element ID before use.

Copilot uses AI. Check for mistakes.
bool inside_cell = false;

auto nearest_element_id = candidates_begin;
auto dimensionality = GridPointSearch2D::Result::Dimensionality::REGION;
Copy link

Copilot AI Nov 16, 2025

Choose a reason for hiding this comment

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

The initial dimensionality is set to REGION, which is a 3D concept (tetrahedron). In 2D, this should be initialized to a more appropriate value. Since the search prioritizes vertex → edge → face in that order, and we're looking for the nearest element, consider initializing to FACE (the highest dimensionality in 2D) or keeping track of whether any element was found at all.

Suggested change
auto dimensionality = GridPointSearch2D::Result::Dimensionality::REGION;
auto dimensionality = GridPointSearch2D::Result::Dimensionality::FACE;

Copilot uses AI. Check for mistakes.
@jacobmerson
Copy link
Collaborator

This PR is blocked by #227 because we need to test on the GPU.

matthew-mccall and others added 21 commits December 10, 2025 10:46
Updated `GridPointSearch` to use a templated design (`GridPointSearch<dim>`) and introduced `GridPointSearch2D` as a specific case for 2D point localization. This change ensures clearer extensibility for other dimensions and improves code maintainability by centralizing shared logic in a new base class (`PointLocalizationSearch`). Adjusted test cases, headers, and related implementations accordingly.
Replaced `std::optional` with `std::unique_ptr` for `search_` to better handle polymorphism and align with future extensions. Updated type aliasing and constructors accordingly to enhance code clarity and maintainability.
Introduced support for 3D point localization and grid intersection with a new `GridTriIntersectionFunctor3D`. Refactored 2D implementations for consistency, generalizing methods to handle dimensions dynamically. Added templates and utilities to support both 2D and 3D use cases effectively.
Replaced the local implementation of barycentric_from_global with Omega_h's standard implementation. Removed the custom function from both source and header files, and updated all calls to use Omega_h::barycentric_from_global<2, 2>. This change enhances code maintainability and aligns functionality with the Omega_h library.
Introduce bbox_verts_within_simplex to check if an AABBox's vertices are within a simplex using barycentric coordinates. Also, add a virtual destructor to the PointLocalizationSearch class for proper cleanup in derived classes.
Renamed `construct_intersection_map` to `construct_intersection_map_2d` and added `construct_intersection_map_3d` for 3D support. Introduced `GridPointSearch3D` class, extending point localization for 3D grids with associated functionality and data structures. Updated tests to reflect these changes.
Introduce a new REGION dimensionality to represent 3D regions in point search operations. Adjust loops and result assignment logic to incorporate this new dimension for improved handling of 3D cases.
Simplified indentation and adjusted line breaks in edge and vertex search loops for improved readability. Added constructor overloads to `GridPointSearch2D` for handling tolerances via a new `PointSearchTolerances` abstraction. Updated `PointLocalizationSearch` class with support for tolerances.
Previous algorithm checked edges then vertices. Now we check vertices, then edges.
Converted `PointSearchTolerances` to use `const` references for safer and more efficient handling. Added default tolerances initialization in 2D and 3D grid search constructors. Renamed `tri_id` to `element_id` in `Result` for improved clarity and consistency across dimensionalities.
…ces handling

Revised result assignment logic to incorporate detailed checks for vertex, edge, and face proximity, using new tolerances-based conditions. Updated tests to reflect changes in dimensionality and result handling for improved accuracy verification.
Removed unused `edge_found` variable for cleaner logic in result assignment. Updated test cases to add new points and verify dimensionality, element IDs, and coordinates for various scenarios, enhancing accuracy and coverage.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Replaced `GridPointSearch` with `GridPointSearch2D` in `omega_h_field2` for improved dimensionality handling.
…` for 2D-specific compatibility improvements and clarity
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