diff --git a/cpp/LinearFactor.h b/cpp/LinearFactor.h index 3d71337e2..dcec35345 100644 --- a/cpp/LinearFactor.h +++ b/cpp/LinearFactor.h @@ -187,6 +187,9 @@ public: /** * Constructor that combines a set of factors + * NOTE: the combined factor will be depends on a system-dependent + * ordering of the input set of factors. Do not rely on this order + * when using the function. * @param factors Set of factors to combine */ CONSTRUCTOR diff --git a/cpp/LinearFactorGraph.h b/cpp/LinearFactorGraph.h index cbab2cc3a..859d03744 100644 --- a/cpp/LinearFactorGraph.h +++ b/cpp/LinearFactorGraph.h @@ -57,12 +57,17 @@ namespace gtsam { /** * find all the factors that involve the given node and remove them * from the factor graph + * NOTE: the ordering of the LinearFactor::shared_ptrs will change + * between systems, so do not rely on this ordering * @param key the key for the given node */ LinearFactorSet find_factors_and_remove(const std::string& key); /** * extract and combine all the factors that involve a given node + * NOTE: the combined factor will be depends on a system-dependent + * ordering of the input set of factors. Do not rely on this order + * when using the function. * @param key the key for the given node * @return the combined linear factor */ diff --git a/cpp/testLinearFactor.cpp b/cpp/testLinearFactor.cpp index b3225a1ad..c418b1be6 100644 --- a/cpp/testLinearFactor.cpp +++ b/cpp/testLinearFactor.cpp @@ -65,10 +65,9 @@ TEST( LinearFactor, variables ) /* ************************************************************************* */ -// NOTE: This test does not pass when running on Linux (Ubuntu 9.04, GCC 4.3.3) -// systems, and appears to be dependent on the ordering of factors in the -// set lfg. Reversing the order in which the matrices are appended will -// make this test work correctly, while the next test will fail. +// NOTE: This test fails due to order dependency in the extraction of factors +// To fix it, it will be necessary to construct the expected version +// of the combined factor taking into account the ordering in the set TEST( LinearFactor, linearFactor2 ) { // create a small linear factor graph diff --git a/cpp/testLinearFactorGraph.cpp b/cpp/testLinearFactorGraph.cpp index 5e9518f26..9c9134194 100644 --- a/cpp/testLinearFactorGraph.cpp +++ b/cpp/testLinearFactorGraph.cpp @@ -153,8 +153,10 @@ TEST( LinearFactorGraph, find_separator ) } /* ************************************************************************* */ -// Note: This test does not pass when running on Linux(Ubuntu 9.04, GCC 4.3.3) -// systems. +// Note: This test fails on different systems due to a dependency on the ordering +// of LinearFactor::shared_ptr in STL sets. Because the ordering is based +// on pointer values instead of factor values, the ordering is different +// between separate systems. TEST( LinearFactorGraph, combine_factors_x1 ) { // create a small example for a linear factor graph @@ -163,6 +165,9 @@ TEST( LinearFactorGraph, combine_factors_x1 ) // combine all factors LinearFactor::shared_ptr actual = fg.combine_factors("x1"); + //FIXME: this expected value must be constructed in the order that + // the factors are stored in a set - which differs between systems + // the expected linear factor Matrix Al1 = Matrix_(6,2, 0., 0., @@ -215,6 +220,9 @@ TEST( LinearFactorGraph, combine_factors_x2 ) // combine all factors LinearFactor::shared_ptr actual = fg.combine_factors("x2"); + // FIXME: change the ordering of the expected to match whatever the real + // ordering is and constructed the expected with the correct order + // the expected linear factor Matrix Al1 = Matrix_(4,2, // l1