-
Notifications
You must be signed in to change notification settings - Fork 901
MultifrontalSolver #2327
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
MultifrontalSolver #2327
Conversation
feat(linear): Finalize MultifrontalSolver implementation and clean up feat(linear): Finalize MultifrontalSolver with benchmarks (Phase 5) feat(linear): Implement in-place back-substitution (Phase 4) feat(linear): Implement imperative multifrontal elimination (Phase 3) feat(linear): Implement iterative value loading (Phase 2) test(linear): Add balanced_smoother test case to MultifrontalSolver tests feat(linear): Implement MultifrontalSolver structure and initialization (Phase 1)
Tighten Clique API Clique compilation unit calculateSeparatorKeys Lazy separatorKeys print methods Make Clique more classy Add streaming
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR introduces a new imperative-style multifrontal solver (MultifrontalSolver) that optimizes Gaussian factor graph elimination by pre-allocating memory and reusing the structure across iterations. The implementation achieves approximately 4x speedup over the standard GTSAM elimination approach for iterative optimization scenarios.
Key changes:
- New
MultifrontalSolverclass that pre-builds the symbolic junction tree and pre-allocates all necessary matrices - New
MultifrontalCliqueclass for imperative clique operations (eliminate, solve) - Extended
VectorValueswith a newinsertmethod for concatenated vectors with explicit key ordering - Added
blockViewmethod toSymmetricBlockMatrixfor non-copying block access
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 23 comments.
Show a summary per file
| File | Description |
|---|---|
gtsam/linear/MultifrontalSolver.h |
Header defining the main MultifrontalSolver class with methods for construction, loading, elimination, and solving |
gtsam/linear/MultifrontalSolver.cpp |
Implementation of MultifrontalSolver including symbolic tree construction and memory pre-allocation |
gtsam/linear/MultifrontalClique.h |
Header for the imperative clique structure with methods for matrix operations and back-substitution |
gtsam/linear/MultifrontalClique.cpp |
Implementation of clique operations including fillAb, eliminateClique, and solveClique |
gtsam/linear/VectorValues.h |
Added new insert method signature for concatenated vectors |
gtsam/linear/VectorValues.cpp |
Implementation of the new insert method that segments a vector by keys and dimensions |
gtsam/base/SymmetricBlockMatrix.h |
Added blockView method for efficient non-copying block access |
gtsam/linear/tests/testMultifrontalSolver.cpp |
Unit tests covering constructor, load, eliminate, and solve operations |
timing/timeMultifrontalSolver.cpp |
Benchmark comparing MultifrontalSolver performance against standard GTSAM elimination |
|
@copilot open a new pull request to apply changes based on the comments in this thread |
Co-authored-by: dellaert <10515273+dellaert@users.noreply.github.com>
- Revert rightHandSideScratch_ back to rhsScratch_ - Change separatorSolutionScratch_ to separatorScratch_ - Remove currentClique reference, use clique-> directly Co-authored-by: dellaert <10515273+dellaert@users.noreply.github.com>
Address code review comments: documentation and naming conventions
| } | ||
| } | ||
|
|
||
| void MultifrontalClique::eliminate() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
eliminateInPlace?
|
I’ll implement both TODOs in follow-up PR |
In order to speed up optimization, I created a fully imperative multifrontal solver that nevertheless is in the style of GTSAM. However, it creates the structure and allocates memory only once, and then can be re-used in subsequent iterations. For a simple chain with 500 nodes, with metis ordering (tree of$\log_2(500)$ deep) it is between 3.5 and 4 times as fast:
Update
After storing pointers into the vectorValues data structure, we are now 5-6 times as fast (on Mac):
Also, I realized GTSAM is using TBB, whereas new solver is single-threaded for now.