testJacobianFactor test fails

[Simon Julier]

The test:

Error in testJacobianFactor (line 59)
CHECK('actualCG.equals(expectedCG,1e-5)',actualCG.equals(expectedCG,1e-4));

fails. (Also there is a mismatch between the reported threshold in the message and the actual threshold used in the test.)

The reason is that actualCG has a noise model assigned to it, but expectedGC does not:

>> actualCG
  Conditional density [1] 
  R = [ 11.1803       0 ]
      [       0 11.1803 ]
  S[2] = [ -2.23607        0 ]
         [        0 -2.23607 ]
  S[3] = [ -8.94427        0 ]
         [        0 -8.94427 ]
  d = [  2.23607 -1.56525 ]
  Noise model: unit (2) 
>> expectedCG
  Conditional density [1] 
  R = [ 11.1803       0 ]
      [       0 11.1803 ]
  S[2] = [ -2.23607        0 ]
         [        0 -2.23607 ]
  S[3] = [ -8.94427        0 ]
         [        0 -8.94427 ]
  d = [  2.23607 -1.56525 ]
  No noise model

Given this, is this a test which should fail? In other words, should it be:

CHECK('~actualCG.equals(expectedCG,1e-4)',~actualCG.equals(expectedCG,1e-4));

[Chris Beall]

This test passes for me on Mac OS and Ubuntu. What platform do you use? And does relaxing the threshold make it pass?

[Simon Julier]

I have seen this on both macOS and Ubuntu. I should point out that I'm using the code off of the develop branch (should this be flagged as a GTSAM 4.0 issue?) I'm also using an update from literally two minutes ago. For Ubuntu, I'm using Matlab 2016a. For OSX I'm using 2016b. On both platforms, the test fails. On Ubuntu, trying to print out the value of expectedCG or actualCG causes matlab to crash.

I don't believe it's a threshold problem - rather the noise model is missing for one of the conditionals. You can actually see this in the output in the original posting (see the last line of the output of expectedCG and actualCG). The equality check fails at lines 106-109 of GaussianConditional.cpp:

      // check if sigmas are equal
      if ((model_ && !c->model_) || (!model_ && c->model_)
        || (model_ && c->model_ && !model_->equals(*c->model_, tol)))
        return false;

I had a look at the test itself, and saw that the models are set according to:

model4 = noiseModel.Diagonal.Sigmas(sigmas);
combined = JacobianFactor(x2, Ax2,  l1, Al1, x1, Ax1, b2, model4);
ord=Ordering;
ord.push_back(x2);
actualCG = combined.eliminate(ord);

expectedCG = GaussianConditional(x2,d,R11,l1,S12,x1,S13);

The constructor for expectedCG is the nargin==7 version and calls the following constructor inside the wrapper:

void gtsamGaussianConditional_constructor_846(int nargout, mxArray *out[], int nargin, const mxArray *in[])
{
  mexAtExit(&_deleteAllObjects);
  typedef boost::shared_ptr<gtsam::GaussianConditional> Shared;

  size_t key = unwrap< size_t >(in[0]);
  Vector d = unwrap< Vector >(in[1]);
  Matrix R = unwrap< Matrix >(in[2]);
  size_t name1 = unwrap< size_t >(in[3]);
  Matrix S = unwrap< Matrix >(in[4]);
  size_t name2 = unwrap< size_t >(in[5]);
  Matrix T = unwrap< Matrix >(in[6]);
  Shared *self = new Shared(new gtsam::GaussianConditional(key,d,R,name1,S,name2,T));
  collector_gtsamGaussianConditional.insert(self);
  out[0] = mxCreateNumericMatrix(1, 1, mxUINT32OR64_CLASS, mxREAL);
  *reinterpret_cast<Shared**> (mxGetData(out[0])) = self;

  typedef boost::shared_ptr<gtsam::GaussianFactor> SharedBase;
  out[1] = mxCreateNumericMatrix(1, 1, mxUINT32OR64_CLASS, mxREAL);
  *reinterpret_cast<SharedBase**>(mxGetData(out[1])) = new SharedBase(*self);
}

I chased up the constructor for the GaussianConditional, but I didn't see anywhere in the path where the noise model appears to be assigned.

[Duy-Nguyen Ta]

@cbeall3 I guess you are talking about the cpp test. The matlab test didn't pass for me as well because the noise models are different. Adding the noise model to the expectedCG will make it pass.

[Simon Julier]

Sorry - I should point out that we're looking at using GTSAM for teaching and the students are all using the matlab interface, and so I'm testing everything through the matlab wrapper.

[Simon Julier]

Is there any progress on this? Is it simply the case that the test needs to be modified to add the noise model?

[Duy-Nguyen Ta]

@SJulier I believe so. Look at EliminateQR and JacobianFactor::splitConditional(...) in JacobianFactor.cpp. The noise model is always added to the conditional. Mathematically, a JacobianFactor without a noise model equals to the one with the Unit noise. So another way is to modify JacobianFactor::equals(...) to do that. However, that seems dangerous to me as "equals" in unit tests might be meant to enforce equality at the object level.

[Frank Dellaert]

Simon, will you make this change as well? you can just include it in the other PR...

[Mustafa Mukadam]

Hi @SJulier, were you able to make a PR for this?

[Frank Dellaert]

Merged in fix/matlab_tests (pull request #361)

close issue #403 close issue #402 close issue #398 close issue #397 close issue #395 close issue #305 close issue #282 close issue #16

Fix/matlab tests Approved-by: Mike Sheffler [e-mail]

→ <<cset b5a878d2af18>>