Skip to content

Commit

Permalink
Throw an error instead of partially implementing it.
Browse files Browse the repository at this point in the history
  • Loading branch information
fdarricau committed May 13, 2015
1 parent 11b8851 commit 86c0263
Show file tree
Hide file tree
Showing 2 changed files with 15 additions and 15 deletions.
7 changes: 5 additions & 2 deletions include/roboptim/core/manifold-map/decorator/dispatcher.hh
Original file line number Diff line number Diff line change
Expand Up @@ -81,9 +81,12 @@ namespace roboptim
}
}

static void applyDiff(const FunctionOnManifold<U>* instance)
static void applyDiff(const FunctionOnManifold<U>*)
{
instance->tangentMappedJacobian = instance->mappedJacobian_;
// The throw will need to be replaced by something useful once the Sparse
// support is ready in libmanifolds.

throw std::string("No Sparse support for non RealSpaces manifolds");

This comment has been minimized.

Copy link
@bchretien

bchretien May 13, 2015

Member

Wait.. 3 things:

  • Dont't throw a std::string! Exceptions are here for a reason.
  • Why don't you check for the sparsity before throwing? The exception here should only be thrown in a sparse scenario with non-R^n manifolds, shouldn't it?
  • Don't throw a std::string!

This comment has been minimized.

Copy link
@bchretien

bchretien May 13, 2015

Member

Ok, this is in a sparse struct, so only the type of manifold needs to be checked (possibly once only?).

}
};
} // end of namespace roboptim
Expand Down
23 changes: 10 additions & 13 deletions tests/manifold-map.cc
Original file line number Diff line number Diff line change
Expand Up @@ -246,25 +246,25 @@ BOOST_AUTO_TEST_CASE_TEMPLATE (manifold_map_test_0, T, functionTypes_t)
}

(*output) << instWrap << "\n";
std::cout << instWrap << std::endl;
std::cout << "input: " << input.transpose() << std::endl;
instWrap(result, input);
std::cout << "result: " << result.transpose() << std::endl << std::endl;

for (int i = 0; i < 10; ++i)
{
std::cout << "i: " << i << std::endl;
instWrap.gradient(gradient, input, i);
}

instWrap.jacobian(jacobian, input);
std::cout << "jacobian: " << std::endl << jacobian << std::endl;
instWrap.manifold_jacobian(refJacobian, input);

try
{
instWrap.manifold_jacobian(refJacobian, input);
}
catch(std::string s)
{
std::cout << s << std::endl;
}
(*output) << descWrapPtr;

std::cout << output->str() << std::endl;
BOOST_CHECK (output->match_pattern());

}

BOOST_AUTO_TEST_CASE_TEMPLATE (manifold_map_test_1, T, functionTypes_t)
Expand Down Expand Up @@ -314,7 +314,6 @@ BOOST_AUTO_TEST_CASE_TEMPLATE (manifold_map_test_1, T, functionTypes_t)
BOOST_CHECK(result(0) == (3 * (3 * i + 1)));

instWrap.jacobian(jacobian, input);
std::cout << to_dense(jacobian) << std::endl;
(*output) << to_dense(jacobian) << std::endl;
}

Expand All @@ -323,6 +322,7 @@ BOOST_AUTO_TEST_CASE_TEMPLATE (manifold_map_test_1, T, functionTypes_t)
delete reals[i];
}

std::cout << output->str() << std::endl;
BOOST_CHECK (output->match_pattern());
}

Expand Down Expand Up @@ -432,8 +432,6 @@ BOOST_AUTO_TEST_CASE_TEMPLATE (manifold_map_test_3, T, functionTypes_t)

(*output) << instWrap;

std::cout << "result: " << result << std::endl;

BOOST_CHECK (result(0) == 15);
BOOST_CHECK (result(1) == 15);
BOOST_CHECK (result(2) == 15);
Expand All @@ -446,7 +444,6 @@ BOOST_AUTO_TEST_CASE_TEMPLATE (manifold_map_test_3, T, functionTypes_t)

for (int i = 0; i < jac.rows(); ++i)
{
std::cout << jac.row(i) << std::endl;
(*output) << jac.row(i) << std::endl;
}

Expand Down

0 comments on commit 86c0263

Please sign in to comment.