iSAM2 and smartfactors about gtsam HOT 4 CLOSED

@dellaert Brilliant! Next week, hopefully will come back to this important one.
Cheers.

from gtsam.

Work in progress here.

On this:

One things is that we definitely want to be backwards compatible on update, including in wrappers.

Let me know if this solution (100% backwards compatible) seems better.
Next, we can add to that structure the new parameters needed to solve this bug.

On this:

“In theory, even smart-factors know which keys they are connected to, so I am not sure why simply giving the keys is not an option.”

It would be great to do it like that; last time I spent some time with this, though, it looked like it was not possible, but will give it another try just in case I missed something.

from gtsam.

I'm super happy this is solved, yay!

@dellaert Please, take a look at the proposed solution in the PR #25.

“In theory, even smart-factors know which keys they are connected to, so I am not sure why simply giving the keys is not an option.”

Perhaps there's another way, but I though that for iSAM2::getAffectedFactors() to work properly:

we need variableIndex_ to be up-to-date, and that implies requesting from the user this information, that's the reason of the new parameter to update().

from gtsam.

Excellent, I will review...

from gtsam.