Copy/paste bug in GaussianConditional.cpp?

[Frank Dellaert]

Could it be that the copy/paste coding between GaussianConditional::solve and GaussianConditional::solveOtherRHS led to a bug? It seems solveOtherRHS has a scaling by sigmas in line 163 which solve does not have...

[Richard Roberts]

Frank, I think that solve is actually correct while solveOtherRHS unncecessarily scales by sigmas. A GaussianConditional describes a perfectly-constrained system with a zero-residual solution, so scaling by sigmas does not affect the solution. It's only in a full system Jacobian, before variable elimination, that we're doing least-squares and thus the sigmas affect the solution. You can see this by algebra too:

Here Sigma cannot be removed:
argmin|| A*x - b ||^2_Sigma  -->  argmin (A*x-b)^T * inv(Sigma) * (A*x-b)

But here SigmaR cancels out because it's invertible:
SigmaR * R*x = SigmaR * d
R*x = d

[Frank Dellaert]

OK, but solveOtherRHS might have it for a reason. Who added it?

[Yong-Dian Jian]

I think it can be used for preconditioning, not sure who added it though.

[Richard Roberts]

I know either you or I added it Yong-Dian, because you needed it for preconditioning.

[Richard Roberts]

I don't think the sigmas are needed for this function either because they should cancel out the same way. What do you think?

[Frank Dellaert]

I think the function is now used, and (hopefully) unit tested with that change. It could be that the RHS is interpreted as not having been divided by the sigmas.

[Richard Roberts]

I don't actually understand what you mean with your response, but sometime over the next week or two I can unit test this to answer it once and for all.