Skip to content

Improve nurbs surface fitting efficiency with Eigen sparse matrix solver#6379

Open
ZhuLingfeng1993 wants to merge 1 commit intoPointCloudLibrary:masterfrom
ZhuLingfeng1993:support-nurbs-solve-eigen-sparse
Open

Improve nurbs surface fitting efficiency with Eigen sparse matrix solver#6379
ZhuLingfeng1993 wants to merge 1 commit intoPointCloudLibrary:masterfrom
ZhuLingfeng1993:support-nurbs-solve-eigen-sparse

Conversation

@ZhuLingfeng1993
Copy link
Contributor

Compared to the existing Eigen dense solver, using the Eigen sparse solver significantly reduces solution time and memory usage, without relying on UMFPACK/SuiteSparse libraries that are based on the GPL open source license.

Compared to the existing Eigen dense solver, using the Eigen sparse solver significantly reduces solution time and memory usage, without relying on UMFPACK/SuiteSparse libraries that are based on the GPL open source license.
@mvieth mvieth added changelog: enhancement Meta-information for changelog generation module: surface labels Dec 20, 2025
@mvieth
Copy link
Member

mvieth commented Dec 20, 2025

Thank you for your pull request.
Can you give some performance measurement, how much faster your proposed new solver is, compared to the existing dense solver? (Ideally also compared to the SuiteSparse solver)
Do you know if there already is a test (e.g. in test/surface/...) that covers your new code?

@ZhuLingfeng1993
Copy link
Contributor Author

Efficiency test result of pcl::on_nurbs::FittingSurface:

Scene number ROI(m) input point number nurbs refinement(XxY) Time with dense solver(s) Memory with dense solver(MB) Time with sparse solver(s) Time of sparse solve function(s) Memory with sparse solver(MB)
1 120x120 6051 5x5 3.9 300 1.24 0.03 306
2 220x194 19094 6x6 182.6 1771 11.8 0.08 521
3 260x154 25313 7x6 1062.6 4871 30.1 0.22 618
4 430x130 27990 7x6 1321.2 5115 28.7 0.15 1030

Note of the table:

  1. dense solver: solve with Eigen::CompleteOrthogonalDecomposition
  2. sparse solver: solve with Eigen::SparseLU
  3. Time of sparse solve function: time of NurbsSolve::solve() using Eigen::SparseLU

Sinces the time of sparse solve funtion is small, I think no need to compare to SuiteSparse solver.

And I find there is no tests for on_nurbs in test/surface, I just test my commit by my own private application code.

@larshg
Copy link
Contributor

larshg commented Feb 3, 2026

It works well with the nurbs fitting example and when run on the bun0.pcd (small cloud) the performance increase is still nice, from about 5 secs to about 800ms.
It would be nice to be able to swap between them, without having to recompile PCL, but that would require more work.
For now its fine for me to merge - it at least gives the option.

@mvieth
Copy link
Member

mvieth commented Feb 4, 2026

@larshg Could it make sense to convert the nurbs fitting example to a test? Since the new sparse solver is used by default, I think it is important to have at least some kind of test to make sure it works correctly.

@larshg
Copy link
Contributor

larshg commented Feb 5, 2026

The unittest from #6407 also passes with this new sparse solver. But we can merge the unittest first and then rebase this afterwards?

@larshg larshg added this to the pcl-1.16.0 milestone Feb 5, 2026
@larshg
Copy link
Contributor

larshg commented Feb 5, 2026

@ZhuLingfeng1993 can you rebase this on main, thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

changelog: enhancement Meta-information for changelog generation module: surface

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants