New functionality in linear

[dellaert]

• Added a likelihood method in GaussianConditional
• Added an html method in VectorValues:

[gchenfc]

Looks good to me. Minor comment about extra copies, but probably not that significant anyway.

[gchenfc]

By this comment, do you mean hopefully the compiler optimizes it out?

Because indeed I think the lines:
auto newAb = Ab_
and
return boost::make_shared<JacobianFactor>(newKeys, newAb, model_);
are both making copies.

Although I guess since the JacobianFactor constructor is copying anyway, I can't think of a functional-style way to avoid the extra copy.

[dellaert]

The first one is for sure, but maybe the second one is optimized out? A la RVO?

[gchenfc]

Hmm I don't think RVO can optimize-out a copy into a function call argument, only for a return value out (i.e. shared_ptr won't have extra copy), but ofc I'm not very knowledgable here.

Though I tried making a quick mock-up:
https://godbolt.org/z/Wze8EW5x3

I think the place the copy is actually happening is the JacobianFactor constructor:

  JacobianFactor::JacobianFactor(
    const KEYS& keys, const VerticalBlockMatrix& augmentedMatrix, const SharedDiagonal& model) :
  Base(keys), Ab_(augmentedMatrix)

attention: Ab_(matrix)
The const VerticalBlockMatrix & ensures there's not a copy there, and I don't know if the standard allows introspecting that far to check to optimize-out a copy

Well in any case, I don't know the copy can be easily avoided regardless, since newAb has to get modified at some point without changing Ab_.

[ProfFan]

Note I think if you do Obj(m): m(std::move(m)) {} the move is guaranteed.

[dellaert]

Thanks @ProfFan . But how do we say that we want to move this copy?