Possibly uninitialized warnings

Issue #387 resolved
Frank Dellaert created an issue

There a few warnings left on latest pipeline build: https://bitbucket.org/gtborg/gtsam/addon/pipelines/home#!/results/66

All tests pass so not an urgent thing. Assigning to @andreicostinescu as he proved particularly adept at solving some of these (I still owe you a beer, Andrei!)

Comments (8)

  1. Andrei Costinescu

    It seems that on the latest develop version, the first warning does not occur anymore.

    I ran cmake .. and cmake -DGTSAM_USE_SYSTEM_EIGEN=OFF -DGTSAM_USE_EIGEN_MKL=OFF .. and the first warning does not appear. It could be that it was solved in the latest Eigen upgrade?

    The second warning still appears, so I'll work on that.

  2. Andrei Costinescu

    I found out where the “error” lies. If you change line 111 in ImuFactorExample2.cpp from auto measuredAcc to Vector3 measuredAcc, the warning disappears. I don’t understand why auto-ing the declaration created these warnings...

  3. Andrei Costinescu

    How do you want me to fix this issue?

    Edit: I think the underlying problem lies somewhere in the file Scenario.h because if you add/subtract anything to the omega_b method in line 112, a similar warning appears for that line.

    Edit2: I thought that the problem was in the ConstantTwistScenario (because that is the initialized scenario in ImuFactorExample2). Specifically, in its constructor, when twist_ gets initialized with an empty Vector6(). However, even after setting the twist_ to a constant 0-valued vector, the warnings still appeared. I then tried modifying the omega_b method in the ConstantTwistScenario to return a constant Vector3(1, 1, 1). Still the same error appeared when I subtracted 0 from scenario.omega_b(t) before assigning it to auto measuredOmega. My conclusion from this is that the gtsam code (implementation) is correct, containing itself no warnings. However, there is a problem with Eigen when dealing with auto.

    Is it ok if I declare measuredAcc and measuredOmega as Vector3 instead of auto?

  4. Frank Dellaert reporter

    Just change it to Vector3 and do a PR? But pull develop first, as I merged a recent change that did something similar.

  5. Log in to comment