-
Notifications
You must be signed in to change notification settings - Fork 769
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
User-provided hook within the internal loop of non-linear optimizers #586
Conversation
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.
LGTM but requesting some minor updates.
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.
LGTM!
* }; | ||
* \endcode | ||
* or to the result of a properly-formed `std::bind` call. | ||
*/ |
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.
Amazing!!!
boost::optional<Ordering> ordering; ///< The optional variable elimination ordering, or empty to use COLAMD (default: empty) | ||
IterativeOptimizationParameters::shared_ptr iterativeParams; ///< The container for iterativeOptimization parameters. used in CG Solvers. | ||
|
||
NonlinearOptimizerParams() = default; |
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.
I remember seeing a comment from @dellaert about having variables before constructors, so he'll be happy about this one. 😄
I believe the build is failing because Actions got rid of Boost 1.69 on Linux virtual environments, opting instead for 1.72. We already landed the PR to upgrade the workflows, but I guess this PR was created before that. I'll push a merge. |
@dellaert Should we merge this one? Do you want to take a look first? |
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.
Nice! Feel free to merge
See the example in the new unit test.
This allows progress reports, debugging, GUI updating, etc. during non-linear optimizers are running.
Note: this PR builds upon #585 .