Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

GJK/EPA: fix epsilon in asserts #554

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ All notable changes to this project will be documented in this file.
The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).

## [Unreleased]
- Fixed too low tolerance in GJK/EPA asserts ([#554](https://github.com/humanoid-path-planner/hpp-fcl/pull/554))
- Fixed `normal_and_nearest_points` test (no need to have Eigen 3.4) ([#553](https://github.com/humanoid-path-planner/hpp-fcl/pull/553))
- [#549](https://github.com/humanoid-path-planner/hpp-fcl/pull/549)
- Optimize EPA: ignore useless faces in EPA's polytope; warm-start support computation for `Convex`; fix edge-cases witness points computation.
Expand Down
14 changes: 7 additions & 7 deletions include/hpp/fcl/narrowphase/narrowphase.h
Original file line number Diff line number Diff line change
Expand Up @@ -199,8 +199,8 @@ struct HPP_FCL_DLLAPI GJKSolver {
}
HPP_FCL_COMPILER_DIAGNOSTIC_POP

static constexpr FCL_REAL dummy_precision =
std::numeric_limits<FCL_REAL>::epsilon() * 100;
const FCL_REAL dummy_precision =
3 * std::sqrt(std::numeric_limits<FCL_REAL>::epsilon());
HPP_FCL_UNUSED_VARIABLE(dummy_precision);
switch (gjk.status) {
case details::GJK::DidNotRun:
Expand Down Expand Up @@ -410,11 +410,11 @@ struct HPP_FCL_DLLAPI GJKSolver {
// In any case, `gjk.ray`'s norm is bigger than GJK's tolerance and thus
// it can safely be normalized.
distance = gjk.distance;
static constexpr FCL_REAL dummy_precision =
std::numeric_limits<FCL_REAL>::epsilon() * 100;
const FCL_REAL dummy_precision =
3 * std::sqrt(std::numeric_limits<FCL_REAL>::epsilon());
HPP_FCL_UNUSED_VARIABLE(dummy_precision);
HPP_FCL_ASSERT(
gjk.ray.norm() > gjk.getTolerance() + dummy_precision,
gjk.ray.norm() > gjk.getTolerance() - dummy_precision,
"The norm of GJK's ray should be bigger than GJK's tolerance.",
std::logic_error);
normal.noalias() = -tf1.getRotation() * gjk.ray;
Expand All @@ -441,8 +441,8 @@ struct HPP_FCL_DLLAPI GJKSolver {
FCL_REAL& distance, Vec3f& p1,
Vec3f& p2,
Vec3f& normal) const {
static constexpr FCL_REAL dummy_precision =
std::numeric_limits<FCL_REAL>::epsilon() * 100;
const FCL_REAL dummy_precision =
3 * std::sqrt(std::numeric_limits<FCL_REAL>::epsilon());
HPP_FCL_UNUSED_VARIABLE(dummy_precision);
HPP_FCL_ASSERT(gjk.distance <= gjk.getTolerance() + dummy_precision,
"The distance should be lower than GJK's tolerance.",
Expand Down
16 changes: 8 additions & 8 deletions src/narrowphase/gjk.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1120,10 +1120,8 @@ bool GJK::projectTetrahedraOrigin(const Simplex& current, Simplex& next) {
const Vec3f a_cross_b = A.cross(B);
const Vec3f a_cross_c = A.cross(C);

// dummy_precision is 1e-14 if FCL_REAL is double
// 1e-5 if FCL_REAL is float
const FCL_REAL dummy_precision(100 *
std::numeric_limits<FCL_REAL>::epsilon());
const FCL_REAL dummy_precision(
3 * std::sqrt(std::numeric_limits<FCL_REAL>::epsilon()));
HPP_FCL_UNUSED_VARIABLE(dummy_precision);

#define REGION_INSIDE() \
Expand Down Expand Up @@ -1747,8 +1745,10 @@ EPA::Status EPA::evaluate(GJK& gjk, const Vec3f& guess) {
// the support we just added is in the direction of the normal of
// the closest_face. Therefore, the support point will **always**
// lie "after" the closest_face, i.e closest_face.n.dot(w.w) > 0.
assert(closest_face->n.dot(w.w) > 0 &&
"The support is not in the right direction.");
if (iterations > 0) {
assert(closest_face->n.dot(w.w) > -tolerance &&
"The support is not in the right direction.");
}
//
// 1) First check: `fdist` (see below) is an upper bound of how much
// more penetration depth we can expect to "gain" by adding `w` to EPA's
Expand Down Expand Up @@ -1921,8 +1921,8 @@ bool EPA::expand(size_t pass, const SimplexVertex& w, SimplexFace* f, size_t e,
// recursive nature of `expand`, it is safer to go through the first case.
// This is because `expand` can potentially loop indefinitly if the
// Minkowski difference is very flat (hence the check above).
const FCL_REAL dummy_precision(100 *
std::numeric_limits<FCL_REAL>::epsilon());
const FCL_REAL dummy_precision(
3 * std::sqrt(std::numeric_limits<FCL_REAL>::epsilon()));
const SimplexVertex& vf = sv_store[f->vertex_id[e]];
if (f->n.dot(w.w - vf.w) < dummy_precision) {
// case 1: the support point is "below" `f`.
Expand Down
Loading