FilterMLS performance optimizations

[jlblancoc]

Summary by CodeRabbit

• Refactor
  • Optimized MLS surface processing with faster weight calculations and significantly reduced temporary memory allocations.
  • Enhanced parallel processing efficiency with improved thread-safe memory management for concurrent operations.
  • Streamlined point projection calculations through caching mechanisms to eliminate redundant computations.
  • Reorganized internal memory handling to support safer, more efficient threaded execution.

[coderabbitai]

Warning

Rate limit exceeded

@jlblancoc has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 14 minutes and 59 seconds before requesting another review.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

📝 Walkthrough

Walkthrough

The changes optimize MLS surface computation in FilterMLS by introducing workspace buffer reuse, thread-local storage via ThreadWorkspace, and power caching mechanisms. Function signatures are extended to accept external workspace parameters, replacing per-iteration allocations with reusable buffers and improving memory efficiency during parallel processing.

Changes

Cohort / File(s)	Summary
Weight and Gaussian Optimization mp2p_icp_filters/src/FilterMLS.cpp	Introduced fast_weight_gaussian function to replace std::exp calls in MLS weight calculations for performance improvement.
Workspace and Buffer Management mp2p_icp_filters/src/FilterMLS.cpp	Added ThreadWorkspace struct for thread-local storage of neighbor data, results, and polynomial/workspace caches. Implemented tbb::enumerable_thread_specific container with single shared workspace fallback when TBB unavailable.
Function Signature Updates mp2p_icp_filters/src/FilterMLS.cpp	Extended computeMLSSurface to accept external workspace buffers (weights, points, basis, u_pow, v_pow). Extended projectPointSimple to receive u_pow_cache and v_pow_cache parameters. Removed EIGEN_MAKE_ALIGNED_OPERATOR_NEW from MLSResult.
MLS Computation Refactoring mp2p_icp_filters/src/FilterMLS.cpp	Reworked surface fitting to operate with explicit workspace buffers. Replaced per-iteration A, b, w matrix construction with direct AtWA and AtWb accumulation. Added per-point power caching for u and v coordinates to reduce redundant calculations.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Poem

🐰 Thread-local workspaces bloom so bright,
Power caches cached just right,
No more allocations with each breath,
Buffers reused till the very death!
Gaussian weights now fly so fast,
Performance gains will surely last! ✨

🚥 Pre-merge checks | ✅ 3

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'FilterMLS performance optimizations' directly and accurately captures the main focus of the changeset, which involves memory reuse, workspace management, and efficiency improvements to the FilterMLS component.
Docstring Coverage	✅ Passed	Docstring coverage is 83.33% which is sufficient. The required threshold is 80.00%.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch feat/mls-optimizations

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 2

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

mp2p_icp_filters/src/FilterMLS.cpp (1)

234-239: ⚠️ Potential issue | 🟠 Major

Bug: LLT decomposition is computed twice, and the check may not reflect the solve's status.

AtWA.llt().solve(AtWb) creates a temporary LLT decomposition, then AtWA.llt().info() creates a second, separate decomposition. The status check doesn't verify the decomposition used for solving.

🐛 Proposed fix: reuse the LLT object

-            // Solve (A^T*W*A)*c = A^T*W*b using Cholesky decomposition
-            c_vec = AtWA.llt().solve(AtWb);
-            if (AtWA.llt().info() != Eigen::Success)
+            // Solve (A^T*W*A)*c = A^T*W*b using Cholesky decomposition
+            Eigen::LLT<Eigen::MatrixXd> llt(AtWA);
+            if (llt.info() != Eigen::Success)
             {
                 return;
             }
+            c_vec = llt.solve(AtWb);

🤖 Fix all issues with AI agents

In `@mp2p_icp_filters/src/FilterMLS.cpp`:
- Around line 50-64: The Padé approximation in fast_weight_gaussian produces
negative weights for large sqr_dist because x = -sqr_dist/sqr_radius can go
below -2; update fast_weight_gaussian to prevent negative returns by clamping
the approximation to a non‑negative value—e.g., after computing x (or xh) check
if x < -2.0 and return 0.0 (or otherwise cap the computed weight with max(0.0,
approx)); keep the existing early exit for very small weights but replace the
unsafe approximation with a clamp to zero for x < -2 to ensure weights used in
the weighted mean/covariance remain nonnegative.
- Around line 366-368: The ThreadWorkspace currently hardcodes buffer sizes
(basis_vector, u_pow_cache, v_pow_cache) for polynomial_order == 3 which leads
to OOB if params.polynomial_order > 3; update initialize_filter (or the
ThreadWorkspace ctor) to validate params.polynomial_order and resize these
buffers dynamically based on polynomial_order: set basis_vector.size() =
(p+1)*(p+2)/2 and u_pow_cache.size(), v_pow_cache.size() = p+1 (where p =
params.polynomial_order), or fail fast with a clear error if an unsupported
order is provided; ensure computeMLSSurface and projectPointSimple rely on these
dynamically sized buffers.

🧹 Nitpick comments (1)

mp2p_icp_filters/src/FilterMLS.cpp (1)

101-103: Remove commented-out code.

These commented members are no longer needed with the workspace-based approach. Leaving them clutters the code.

♻️ Suggested removal

-    // mutable std::vector<double> u_pow;  // Reusable workspace
-    // mutable std::vector<double> v_pow;  // Reusable workspace
-

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 77.07%. Comparing base (2bfd9a8) to head (2ccd5a7).

Additional details and impacted files

@@             Coverage Diff             @@
##           develop      #38      +/-   ##
===========================================
+ Coverage    76.99%   77.07%   +0.07%     
===========================================
  Files          179      179              
  Lines         9529     9557      +28     
  Branches       917      919       +2     
===========================================
+ Hits          7337     7366      +29     
+ Misses        2192     2191       -1     

Files with missing lines	Coverage Δ
mp2p_icp_filters/src/FilterMLS.cpp	95.86% <100.00%> (+1.00%)	⬆️

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.