Refactor and optimize alignment and density map code#114
Merged
saurabh1002 merged 22 commits intomainfrom Apr 27, 2026
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR refactors and tightens the map-closure pipeline across C++ (alignment / ground alignment / density maps) and Python (pipeline, datasets), alongside packaging metadata and license header updates.
Changes:
- Refactors 2D RANSAC alignment, ground alignment, voxel map, and density-map discretization logic for clearer math and reduced overhead.
- Updates Python pipeline outputs (saving ground alignment transforms + HBST database) and shifts quaternion handling from
pyquaterniontoscipy. - Updates packaging metadata (Python version floor, dependency pinning, classifiers) and refreshes copyright/license headers.
Reviewed changes
Copilot reviewed 37 out of 37 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
| python/pyproject.toml | Updates packaging metadata, Python version requirement, dependency ranges, and classifiers. |
| python/map_closures/voxel_map.py | Updates license header. |
| python/map_closures/visualizer/visualizer.py | Updates license header. |
| python/map_closures/visualizer/registration_visualizer.py | Updates license header. |
| python/map_closures/visualizer/local_maps_visualizer.py | Updates license header. |
| python/map_closures/visualizer/closures_visualizer.py | Updates license header. |
| python/map_closures/tools/utils.py | Adds MIT license header to utilities module. |
| python/map_closures/tools/evaluation.py | Adjusts evaluation reporting/table formatting and typing. |
| python/map_closures/tools/cmd.py | Updates license header. |
| python/map_closures/tools/init.py | Updates license header. |
| python/map_closures/pybind/map_closures_pybind.cpp | Updates license header. |
| python/map_closures/pybind/CMakeLists.txt | Updates license header. |
| python/map_closures/pipeline.py | Renames variables for clarity; adds persisted outputs (ground transforms, database) and richer console reporting. |
| python/map_closures/map_closures.py | Updates license header. |
| python/map_closures/datasets/ncd.py | Switches quaternion-to-rotation conversion to SciPy. |
| python/map_closures/datasets/helipr.py | Adds .ply support via Open3D and keeps .bin fallback. |
| python/map_closures/datasets/apollo.py | Switches quaternion-to-rotation conversion to SciPy. |
| python/map_closures/config/config.py | Updates license header. |
| python/map_closures/config/init.py | Updates license header. |
| python/map_closures/init.py | Updates license header. |
| python/LICENSE | Updates copyright line. |
| python/CMakeLists.txt | Updates license header. |
| cpp/map_closures/VoxelMap.hpp | Refactors constants and introduces precomputed sqrt for map resolution. |
| cpp/map_closures/VoxelMap.cpp | Refactors mean/covariance computation and container usage for efficiency/clarity. |
| cpp/map_closures/MapClosures.hpp | Refactors closure retrieval and adds a persistent BFMatcher member. |
| cpp/map_closures/MapClosures.cpp | Refactors ORB self-matching flow and improves top-k selection logic. |
| cpp/map_closures/GroundAlign.hpp | Updates license header. |
| cpp/map_closures/GroundAlign.cpp | Refactors ground sampling/filtering, linear system construction, and optimization loop. |
| cpp/map_closures/DensityMap.hpp | Updates license header. |
| cpp/map_closures/DensityMap.cpp | Fixes 2D discretization blocks and improves pixel counting logic. |
| cpp/map_closures/CMakeLists.txt | Updates license header. |
| cpp/map_closures/AlignRansac2D.hpp | Adds PointPair operators to simplify accumulation logic. |
| cpp/map_closures/AlignRansac2D.cpp | Refactors mean/covariance and inlier checks; parameterizes sample size. |
| cpp/LICENSE | Updates copyright line. |
| cpp/CMakeLists.txt | Updates license header. |
| cpp/3rdparty/find_dependencies.cmake | Updates license header. |
| LICENSE | Updates copyright line. |
Comments suppressed due to low confidence (1)
python/map_closures/tools/evaluation.py:112
rich.Tableneeds columns defined before callingadd_row. Since alladd_column(...)calls were removed, this will raise at runtime (or render incorrectly). Re-introduce the column definitions (and optionally the caption) so the table structure matches the rows being added.
def _rich_table_pr(self, table_format: box.Box = box.HORIZONTALS) -> Table:
table = Table(box=table_format)
for i, metric in enumerate(self.metrics):
table.add_row(
f"{i + 3} {metric.tp} {metric.fp} {metric.fn} {metric.precision:.4f} {metric.recall:.4f} {metric.F1:.4f}"
)
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Agent-Logs-Url: https://github.com/PRBonn/MapClosures/sessions/e336eb2a-b3e4-4578-8d20-33af4cf07afc Co-authored-by: saurabh1002 <28734882+saurabh1002@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Agent-Logs-Url: https://github.com/PRBonn/MapClosures/sessions/b706290d-4bc2-49cf-9ea4-52d48689b110 Co-authored-by: saurabh1002 <28734882+saurabh1002@users.noreply.github.com>
…l dependency Agent-Logs-Url: https://github.com/PRBonn/MapClosures/sessions/65ae623c-2cb0-4347-91f2-cae487a75ac0 Co-authored-by: saurabh1002 <28734882+saurabh1002@users.noreply.github.com>
mehermvr
approved these changes
Apr 27, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This pull request introduces several improvements and refactorings to the map closure and ground alignment modules. The main code changes enhance code clarity, numerical stability, and maintainability, particularly in the 2D RANSAC alignment and ground alignment logic.
Summary of most important changes:
Refactoring and Code Improvements
KabschUmeyamaAlignment2DinAlignRansac2D.cppto use operator overloading forPointPairand improved mean and covariance calculations, leading to clearer and more robust code. Addedoperator+andoperator/toPointPairinAlignRansac2D.hpp.AlignRansac2D.cppby parameterizing sample size withmin_points, using squared distance for inlier checks, and making variable types more explicit.Ground Alignment Enhancements
GroundAlign.cppto define algorithm parameters asconstexpr, improved normal filtering, and enhanced numerical stability and early returns for degenerate cases. Improved the linear system construction and main alignment loop for clarity and correctness.Density Map Calculation Improvements
DensityMap.cppto correctly use 2x3 and 2x1 matrix blocks and improved pixel counting logic for better numerical accuracy.Copyright and License Updates