iSAM2 and smartfactors

[dellaert]

Re-opening bitbucket issue 420

[jlblancoc]

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

[jlblancoc]

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.

[jlblancoc]

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:

gtsam/gtsam/nonlinear/ISAM2.cpp

Lines 109 to 113 in dc774ed

	FactorIndexSet indices;
	for (const Key key : keys) {
	const VariableIndex::Factors& factors(variableIndex_[key]);
	indices.insert(factors.begin(), factors.end());
	}

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().

[dellaert]

Excellent, I will review...