Copy/paste bug in GaussianConditional.cpp?
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...
Comments (7)
-
-
reporter OK, but solveOtherRHS might have it for a reason. Who added it?
-
I think it can be used for preconditioning, not sure who added it though.
-
I know either you or I added it Yong-Dian, because you needed it for preconditioning.
-
I don't think the sigmas are needed for this function either because they should cancel out the same way. What do you think?
-
reporter 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.
-
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.
- Log in to comment
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: