Bmm ros pkg by IasonTheodorou · Pull Request #1 · tue-robotics/bmm_ros

IasonTheodorou · 2025-09-09T17:59:17Z

variational gmm is not used in the code (its an even more advanced implementation for future use (if needed)). So its not fully done.

…odel initial implementation

…CMakeLists (still not working)

… CMakeFile (closer to correct pkg still not working)

…ll trigger exceptions)

Copilot

Pull Request Overview

This PR introduces a new ROS package called "bmm_ros" that implements Bayesian Mixture Models for point cloud denoising. The package provides two different Bayesian approaches: MAP-GMM (Maximum A Posteriori Gaussian Mixture Model) and VB-GMM (Variational Bayes Gaussian Mixture Model) for clustering and outlier detection in 3D point cloud data.

Implements both MAP-GMM and VB-GMM algorithms for point cloud clustering
Provides comprehensive test suite with parameterized tests for quality metrics
Includes synthetic data generation for validation and testing

Reviewed Changes

Copilot reviewed 9 out of 10 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
package.xml	Defines ROS package metadata and dependencies
CMakeLists.txt	Configures build system for library and executable targets
include/bayesian_gmm.h	Header for MAP-GMM implementation with priors
include/variational_gmm.h	Header for VB-GMM implementation
src/bayesian_gmm.cpp	Core MAP-GMM algorithm implementation
src/variational_gmm.cpp	Core VB-GMM algorithm implementation
src/main.cpp	Example application with synthetic data and evaluation
test/bmm_test.cpp	Comprehensive test suite for both algorithms
LICENSE	Custom license for commercial usage restrictions

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

Copilot · 2025-09-09T18:00:44Z

src/bayesian_gmm.cpp

+    for (int k = 0; k < K_; k++) {
+        int idx = rand() % N;
+        means_[k] = data.row(idx).transpose();
+        covs_[k] = Eigen::Matrix3d::Identity() * 0.01;  // Small initial cov NEED TO PUT THIS ON THE PARAMETERS AS WELL


The TODO comment 'NEED TO PUT THIS ON THE PARAMETERS AS WELL' should be addressed or removed. Consider adding a parameter for initial covariance scaling to the GMMParams struct.

Suggested change

covs_[k] = Eigen::Matrix3d::Identity() * 0.01; // Small initial cov NEED TO PUT THIS ON THE PARAMETERS AS WELL

covs_[k] = Eigen::Matrix3d::Identity() * params_.initial_covar_scale; // Small initial cov

Copilot · 2025-09-09T18:00:45Z

src/variational_gmm.cpp

+    // Add other terms for complete lower bound...
+    // (Gaussian-Wishart terms omitted for brevity but should be included)


The lower bound computation is incomplete as indicated by the comment. For a production VB-GMM implementation, the Gaussian-Wishart terms should be included for correct convergence monitoring.

@IasonTheodorou please take a look

Copilot · 2025-09-09T18:00:45Z

LICENSE

+If you wish to use the Software for commercial purposes, you must contact the original developer at [https://www.linkedin.com/in/hamdi-boukamcha/] to obtain a commercial license.
+The terms of any commercial license will be mutually agreed upon and may involve a licensing fee.
+4. Attribution
+
+Regardless of whether you are using the Software for commercial or non-commercial purposes, you must provide appropriate credit to the original developer in any distributions or products that use the Software.
+5. Disclaimer of Warranty
+
+The Software is provided "as is," without warranty of any kind, express or implied, including but not limited to the warranties of merchantability, fitness for a particular purpose, and non-infringement. In no event shall the original developer be liable for any claim, damages, or other liability, whether in an action of contract, tort, or otherwise, arising from, out of, or in connection with the Software or the use or other dealings in the Software.


The license references a different developer (Hamdi Boukamcha) than the package maintainer (Iason Theodorou) listed in package.xml. This inconsistency should be resolved.

Suggested change

If you wish to use the Software for commercial purposes, you must contact the original developer at [https://www.linkedin.com/in/hamdi-boukamcha/] to obtain a commercial license.

The terms of any commercial license will be mutually agreed upon and may involve a licensing fee.

4. Attribution

Regardless of whether you are using the Software for commercial or non-commercial purposes, you must provide appropriate credit to the original developer in any distributions or products that use the Software.

5. Disclaimer of Warranty

The Software is provided "as is," without warranty of any kind, express or implied, including but not limited to the warranties of merchantability, fitness for a particular purpose, and non-infringement. In no event shall the original developer be liable for any claim, damages, or other liability, whether in an action of contract, tort, or otherwise, arising from, out of, or in connection with the Software or the use or other dealings in the Software.

If you wish to use the Software for commercial purposes, you must contact the original developer, Iason Theodorou, at [INSERT CONTACT INFO HERE] to obtain a commercial license.

The terms of any commercial license will be mutually agreed upon and may involve a licensing fee.

4. Attribution

Regardless of whether you are using the Software for commercial or non-commercial purposes, you must provide appropriate credit to the original developer, Iason Theodorou, in any distributions or products that use the Software.

5. Disclaimer of Warranty

The Software is provided "as is," without warranty of any kind, express or implied, including but not limited to the warranties of merchantability, fitness for a particular purpose, and non-infringement. In no event shall the original developer, Iason Theodorou, be liable for any claim, damages, or other liability, whether in an action of contract, tort, or otherwise, arising from, out of, or in connection with the Software or the use or other dealings in the Software.

Copilot · 2025-09-09T18:00:46Z

CMakeLists.txt

+#add_compile_options(-Wall -Werror=all)
+#add_compile_options(-Wextra -Werror=extra)


[nitpick] Consider enabling these warning flags to improve code quality and catch potential issues during compilation.

Suggested change

#add_compile_options(-Wall -Werror=all)

#add_compile_options(-Wextra -Werror=extra)

add_compile_options(-Wall -Werror=all)

add_compile_options(-Wextra -Werror=extra)

src/variational_gmm.cpp

MatthijsBurgh

Fix missing new lines at EOF. License needs to be in name of the TU.

I didn't check the rest yet.

MatthijsBurgh

Update to use hpp header extension. Make sure to add TUe CI.

…t call this can initialize the BMM params

…n for variational bmm

IasonTheodorou · 2025-09-14T11:31:21Z

Update to use hpp header extension. Make sure to add TUe CI.

I will do the .hpp change but why ?

MatthijsBurgh

Fix missing newline at EOF. To prevent the issue in the future, probably you need to change a setting in your IDE.

MatthijsBurgh · 2025-09-15T08:31:02Z

I will do the .hpp change but why ?

Is the newer standard.

MatthijsBurgh

Make sure to place curly brackets on their own line

* Initial plan * Fix curly bracket formatting in all C++ files Co-authored-by: MatthijsBurgh <18014833+MatthijsBurgh@users.noreply.github.com> * Clean up build artifacts and update .gitignore Co-authored-by: MatthijsBurgh <18014833+MatthijsBurgh@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: MatthijsBurgh <18014833+MatthijsBurgh@users.noreply.github.com>

IasonTheodorou

its good

Added notes on future updates and review comments regarding variational Bayesian inference and lower bound computation.

IasonTheodorou added 6 commits September 5, 2025 15:19

Created the bayesian mixture model and variational gaussian mixture m…

9786965

…odel initial implementation

Added files to make it a catkin pkg and created skeleton of main and …

fb63af7

…CMakeLists (still not working)

Improved the pkg with correct dependencies in the package.xml and the…

cfc3524

… CMakeFile (closer to correct pkg still not working)

GMM main can compile and run (still we use empty matrices so Eigen wi…

d20780a

…ll trigger exceptions)

Synthetic data generator for testing (not fully finished yet)

2bb4f97

Added tests and hooks for numerical stability

e8a3360

IasonTheodorou requested review from MatthijsBurgh and Copilot September 9, 2025 17:59

Copilot AI reviewed Sep 9, 2025

View reviewed changes

Removed ed_sensor_integration redundant dependency

385a154

IasonTheodorou force-pushed the bmm_ros_pkg branch from ff2187c to 385a154 Compare September 10, 2025 10:19

MatthijsBurgh requested changes Sep 10, 2025

View reviewed changes

IasonTheodorou added 5 commits September 10, 2025 16:27

Fixed the initializing of BMM params issue. Now any other program tha…

6d3ceb1

…t call this can initialize the BMM params

stored as comment different implementations (db scan vanilla gmm, mai…

9c0f110

…n for variational bmm

Renamed files enabled and fixed warnings and made better comments

f8d50bf

Corrected license and added n line at EOF

d9eb493

Added TuE CI

fe22d4c

IasonTheodorou added 2 commits September 14, 2025 13:37

Used hpp instead of h files

45175cc

Removed redundant things from package and CMakeLists

cacfdbe

MatthijsBurgh requested changes Sep 15, 2025

View reviewed changes

MatthijsBurgh and others added 7 commits September 16, 2025 19:47

Change pcl dep to io only

6f3f033

Add author

350cfb6

Reorder package.xml

005f504

Add missing dep on libpcl-all-dev

e026161

Deleted plc additional includes

3c565c9

Add catkin_lint_cmake

cd717b6

Big cleanup

dab61ac

MatthijsBurgh added 3 commits September 16, 2025 21:27

Fix some cmake linking issues

614858c

Fix header location

bfd7614

Fix missing newline at EOF

58fc921

MatthijsBurgh requested changes Sep 18, 2025

View reviewed changes

IasonTheodorou commented Sep 23, 2025

View reviewed changes

Update README with future feature notes and comments

5b10986

Added notes on future updates and review comments regarding variational Bayesian inference and lower bound computation.

IasonTheodorou requested a review from MatthijsBurgh September 23, 2025 16:07

MatthijsBurgh approved these changes Sep 23, 2025

View reviewed changes

IasonTheodorou merged commit e0114e1 into master Sep 23, 2025
2 checks passed

IasonTheodorou deleted the bmm_ros_pkg branch September 23, 2025 16:18

	covs_[k] = Eigen::Matrix3d::Identity() * 0.01; // Small initial cov NEED TO PUT THIS ON THE PARAMETERS AS WELL
	covs_[k] = Eigen::Matrix3d::Identity() * params_.initial_covar_scale; // Small initial cov

		// Add other terms for complete lower bound...
		// (Gaussian-Wishart terms omitted for brevity but should be included)

		#add_compile_options(-Wall -Werror=all)
		#add_compile_options(-Wextra -Werror=extra)

Comments

Conversation

IasonTheodorou commented Sep 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Copilot AI Sep 9, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Sep 9, 2025

Choose a reason for hiding this comment

Uh oh!

MatthijsBurgh Sep 18, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Sep 9, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Sep 9, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

MatthijsBurgh left a comment

Choose a reason for hiding this comment

Uh oh!

MatthijsBurgh left a comment

Choose a reason for hiding this comment

Uh oh!

IasonTheodorou commented Sep 14, 2025

Uh oh!

MatthijsBurgh left a comment

Choose a reason for hiding this comment

Uh oh!

MatthijsBurgh commented Sep 15, 2025

Uh oh!

MatthijsBurgh left a comment

Choose a reason for hiding this comment

Uh oh!

IasonTheodorou left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

IasonTheodorou commented Sep 9, 2025 •

edited

Loading