Skip to content

Conversation

@MohammadElahiNU
Copy link

• Uniform-grid neighbor finder (sphere & circle)
• New point-to-mesh test using SVD-MLS solver for the MLS interpolation

Copy link
Collaborator

@abhiyanpaudel abhiyanpaudel left a comment

Choose a reason for hiding this comment

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

Overall looks good — just a couple of minor comments.

const double max_coord = pt[d] + cutoff;

min_idx[d] = static_cast<int>((min_coord - grid_obj.bot_left[d]) / cell_size(d));
min_idx[d] = (min_idx[d] < 0) ? 0 : (min_idx[d] >= grid_obj.divisions[d]) ? grid_obj.divisions[d] - 1 : min_idx[d];
Copy link
Collaborator

Choose a reason for hiding this comment

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

This appears to clamp indices to the range [0, grid_obj.divisions[d] - 1]. Adding a comment to clarify the intent would improve readability.

cell_id = x;
}

if (cell_id >= num_cells || cell_ptrs[cell_id] == cell_ptrs[cell_id + 1]) return;
Copy link
Collaborator

Choose a reason for hiding this comment

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

is cell_ptrs.size() == num_cells + 1?

@jacobmerson
Copy link
Collaborator

@MohammadElahiNU @abhiyanpaudel now that the mls_interpolation is using the SVD solver, how much of this code is redundant?

@MohammadElahiNU
Copy link
Author

@MohammadElahiNU @abhiyanpaudel now that the mls_interpolation is using the SVD solver, how much of this code is redundant?

Just one change in point_to_mesh_mls.cpp: drop the test_interpolation_point_to_mesh and replace it with a direct call to mls_interpolation.

@abhiyanpaudel
Copy link
Collaborator

@MohammadElahiNU @abhiyanpaudel now that the mls_interpolation is using the SVD solver, how much of this code is redundant?

I think there are some which are not needed.

@jacobmerson
Copy link
Collaborator

I just went through this and I think there are still some remaining issues to resolve before this gets merged.

@jacobmerson jacobmerson mentioned this pull request Nov 3, 2025
13 tasks
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.

3 participants