load2D fails on victoria_park dataset

[Richard Roberts]

load2D seems to be broken now on the victora_park dataset (the file in examples/Data). It fails with the following error:

load2D::readNoiseModel looks like this is not TORO matrix order

[Siddharth Choudhary]

The default NoiseFormat for load2D is NoiseFormatGRAPH which requires TORO matrix order.

Victoria park instead should be loaded with NoiseFormatCOV.

The following works for me: boost::tie(graph, initial) = load2D(filename, SharedNoiseModel(), 0, false, true, NoiseFormatCOV );

[Frank Dellaert]

Awesome. Please check in a fix with amended error message?

[Richard Roberts]

Hi guys, I had replied to this but apparently it didn't go through, so trying again...

We used to have some auto-detection of the covariance order, but it seems that is not working. This issue comes up in the SolverComparer script, which has command line arguments to specify the dataset. It would be a pain in the butt (and too error-prone) for the user to have to also specify the covariance order as a command line argument. The autodetect code just looked to see where the zeros were, since most of the time the covariance is diagonal. If it couldn't detect, it would throw an error.

[Siddharth Choudhary]

We can add back the following code snippet in dataset.cpp to guess the NoiseFormat instead of using the default noise model.

      // Try to guess covariance matrix layout
      if(v1 != 0.0 && v2 == 0.0 && v3 != 0.0 && v4 != 0.0 && v5 == 0.0 && v6 == 0.0)
      {
// NoiseFormatGRAPH
NoiseFormat noiseFormat = NoiseFormatGRAPH;
      }
      else if(v1 != 0.0 && v2 == 0.0 && v3 == 0.0 && v4 != 0.0 && v5 == 0.0 && v6 != 0.0)
      {
// NoiseFormatCOV
NoiseFormat noiseFormat = NoiseFormatCOV;
      }
      else
      {
        throw std::invalid_argument("load2D: unrecognized covariance matrix format in dataset file. Please specify the noise format.");
      }

What do you think?

[Frank Dellaert]

When was it removed, by whom, and with which commit comment?

[Siddharth Choudhary]

I think you removed it in the commit https://bitbucket.org/gtborg/gtsam/src/0f2d98319052876dcd319b21a83ac8dceb1faf8a/gtsam/slam/dataset.cpp?at=develop while cleaning up noise parameter handling.

[Frank Dellaert]

OK, maybe add a new NoiseFormat value "NoiseFormatAuto" that is the default, which will trigger the above piece of code? Seems it should be under caller control.

[Frank Dellaert]

@itzsid resolved this although I did not see a test-case proving that it actually works.