Fixes #1886
Fixes #1886
Conversation
gtsam/linear/PCGSolver.h
Outdated
| @@ -42,7 +42,7 @@ struct GTSAM_EXPORT PCGSolverParameters : public ConjugateGradientParameters { | |||
| PCGSolverParameters() {} | |||
|
|
|||
| PCGSolverParameters( | |||
| const std::shared_ptr<PreconditionerParameters> &preconditioner) | |||
| const std::shared_ptr<PreconditionerParameters> preconditioner) | |||
There was a problem hiding this comment.
Why? Not in PR comment and not backward compatible
There was a problem hiding this comment.
Backwards compatibility is not an issue here since I introduced this constructor last week. If the Matlab wrapper compiles, I'll add the reference & back.
There was a problem hiding this comment.
Also does a reference to a shared_ptr make sense? The pointer itself is a "reference" and by passing it by value, we encourage the method scope to have ownership and keep the underlying object alive.
There was a problem hiding this comment.
Is it depends on what you want. If you want the underlying object to keep a shared pointer until it is deleted you pass by value; if you know that method will just use the value but has no use for afterwards you pass a reference.
hub_push.shexecutablechi_squared_quantileinline to avoid multiple definitions error.NonlinearConjugateGradientOptimizer.