The estimates should not change if the batch is rejected.

[furgalep]

I'm seeing this here during camera calibration. A new image comes in...it runs the estimation...the image isn't accepted because of the MI threshold, but the estimate is remains what it converged to during the test. This is especially important to avoid problems when bad batches enter the picture.

[jmaye]

This one was on my todo list for a while actually. No other way than saving values of the design variables before optimizing and restoring afterwards. Or one could run the optimizer again after removing the batch.

[furgalep]

Yeah...I realize that. Saving is probably better.

On Sun, Apr 14, 2013 at 1:27 PM, Jerome Maye notifications@github.comwrote:

This one was on my todo list for a while actually. No other way than saving values of the design variables before optimizing and restoring afterwards. Or one could run the optimizer again after removing the batch.

— Reply to this email directly or view it on GitHubhttps://github.com/ethz-asl/aslam_incremental_calibration/issues/10#issuecomment-16349662 .

Paul Furgale

[jmaye]

This will be trickier than initially thought... we have to restore:

• design variables values, don't know yet how to do it since DesignVariable has nothing useful for it
• linear system solver state, because if we want to query R or the rank or other stuff, it has to contain the correct problem in there

[furgalep]

I was thinking about this a long time ago. After implementing a bunch of design variables with the "revertUpdate" function, I got annoyed and started to wish that I had implemented "getParameters(Eigen::VectorXd & P)" and "setParameters(const Eigen::VectorXd & P)". This is similar to what Ceres does as well. Then design variables don't have to manage their own storage of intermediate values.

On Sun, Apr 14, 2013 at 8:45 PM, Jerome Maye notifications@github.comwrote:

This will be trickier than initially thought... we have to restore:

• design variables values, don't know yet how to do it since VectorDesignVariable has nothing useful for it
• linear system solver state, because if we want to query R or the rank or other stuff, it has to contain the correct problem in there

— Reply to this email directly or view it on GitHubhttps://github.com/ethz-asl/aslam_incremental_calibration/issues/10#issuecomment-16356827 .

Paul Furgale

[jmaye]

Ok, I see 2 ways then:

1. Adding this getter/setter functions at the level of DesignVariable, which might be annoying for people using it already. I would mean modifying a bunch of classes.
2. Relying on the copy/assignment constructors. It seems you are not implementing them at all in your classes, thus relying on the default ones. These ones might be sufficient since they are doing a shallow copy of the members.

[jmaye]

Two new functions are now part of the DesignVariable base class:

  /// Returns the content of the design variable
  void getParameters(Eigen::MatrixXd& value) const;

  /// Sets the content of the design variable
  void setParameters(const Eigen::MatrixXd& value);

A derived class should implement these functions in the protected section:

  /// Returns the content of the design variable
  virtual void getParametersImplementation(Eigen::MatrixXd& value)
    const;

  /// Sets the content of the design variable
  virtual void setParametersImplementation(const Eigen::MatrixXd& value);

There is currently no checking going on as to the value we pass in is of the correct dimension, this might be done, but then will slow the code.