commit c1ffad6009294c03d915d4d5617ac3349b22a625
author Sameer Agarwal <sameeragarwal@google.com> Fri Oct 05 16:29:37 2012 -0700
committer Sameer Agarwal <sameeragarwal@google.com> Fri Oct 05 16:58:32 2012 -0700
tree 18d0e9297ea0cee4729a5697e6f489e096d0cbbf
parent e3c5570b0538600a53be0caf863f177aa898fe04

Schur ordering was operating on the original program.

Thanks to Ricardo Martin for reporting this.

Change-Id: Iacf44f672287e4d76633878c7051694a39b3056f

internal/ceres/solver_impl.cc

diff --git a/internal/ceres/solver_impl.cc b/internal/ceres/solver_impl.cc
index 062577e..03fe921 100644
--- a/internal/ceres/solver_impl.cc
+++ b/internal/ceres/solver_impl.cc
@@ -627,7 +627,7 @@
   // maximal independent set.
   if (original_num_groups == 1 && IsSchurType(options->linear_solver_type)) {
     vector<ParameterBlock*> schur_ordering;
-    const int num_eliminate_blocks = ComputeSchurOrdering(*original_program,
+    const int num_eliminate_blocks = ComputeSchurOrdering(*transformed_program,
                                                           &schur_ordering);
     CHECK_EQ(schur_ordering.size(), transformed_program->NumParameterBlocks())
         << "Congratulations, you found a Ceres bug! Please report this error "

internal/ceres/solver_impl_test.cc

diff --git a/internal/ceres/solver_impl_test.cc b/internal/ceres/solver_impl_test.cc
index 5f4a82c..9b01766 100644
--- a/internal/ceres/solver_impl_test.cc
+++ b/internal/ceres/solver_impl_test.cc
@@ -386,6 +386,63 @@
   }
 }
 
+TEST(SolverImpl, AutomaticSchurReorderingRespectsConstantBlocks) {
+  ProblemImpl problem;
+  double x;
+  double y;
+  double z;
+
+  problem.AddParameterBlock(&x, 1);
+  problem.AddParameterBlock(&y, 1);
+  problem.AddParameterBlock(&z, 1);
+
+  // Set one parameter block constant.
+  problem.SetParameterBlockConstant(&z);
+
+  problem.AddResidualBlock(new UnaryCostFunction(), NULL, &x);
+  problem.AddResidualBlock(new BinaryCostFunction(), NULL, &z, &x);
+  problem.AddResidualBlock(new BinaryCostFunction(), NULL, &z, &y);
+  problem.AddResidualBlock(new BinaryCostFunction(), NULL, &z, &y);
+  problem.AddResidualBlock(new BinaryCostFunction(), NULL, &x, &z);
+  problem.AddResidualBlock(new BinaryCostFunction(), NULL, &z, &y);
+  problem.AddResidualBlock(new BinaryCostFunction(), NULL, &x, &z);
+  problem.AddResidualBlock(new UnaryCostFunction(), NULL, &y);
+  problem.AddResidualBlock(new UnaryCostFunction(), NULL, &z);
+
+  ParameterBlockOrdering* ordering = new ParameterBlockOrdering;
+  ordering->AddElementToGroup(&x, 0);
+  ordering->AddElementToGroup(&z, 0);
+  ordering->AddElementToGroup(&y, 0);
+
+  Solver::Options options;
+  options.linear_solver_type = DENSE_SCHUR;
+  options.ordering = ordering;
+
+  string error;
+  scoped_ptr<Program> reduced_program(
+      SolverImpl::CreateReducedProgram(&options, &problem, NULL, &error));
+
+  const vector<ResidualBlock*>& residual_blocks =
+      reduced_program->residual_blocks();
+  const vector<ParameterBlock*>& parameter_blocks =
+      reduced_program->parameter_blocks();
+
+  const vector<ResidualBlock*>& original_residual_blocks =
+      problem.program().residual_blocks();
+
+  EXPECT_EQ(residual_blocks.size(), 8);
+  EXPECT_EQ(reduced_program->parameter_blocks().size(), 2);
+
+  // Verify that right parmeter block and the residual blocks have
+  // been removed.
+  for (int i = 0; i < 8; ++i) {
+    EXPECT_NE(residual_blocks[i], original_residual_blocks.back());
+  }
+  for (int i = 0; i < 2; ++i) {
+    EXPECT_NE(parameter_blocks[i]->mutable_user_state(), &z);
+  }
+}
+
 TEST(SolverImpl, ApplyUserOrderingOrderingTooSmall) {
   ProblemImpl problem;
   double x;