Wrong halfspace intersection

TL;DR

When an edge of a polygon essentially overlaps with the edge of the intersection half-space, bad things happen. Among others, we are searching for an intersection point between two essentially overlapping lines. In code, this results in a determinant in a denominator being essentially zero. I think it would be safer not to remove polygon vertices which are very close to the edge of the halfspace. In code, this would mean replacing a line in ReachPolygon::intersect_halfspace with

bool ret = (a * vec_vertices[i].x + b * vec_vertices[i].y < c + 1e-10);

(note the added + 1e-10 part) or something similar.

The long story - My debugging journey

On a realistic scenario, I was occasionally receiving empty sets as results of the reachability analysis. I chased this down to ReachableSet::_compute_drivable_area_at_step occasionally computing flat sets when propagating a base set. All sets were then pruned, resulting in an empty reachability analysis result.

Wrong propagation

Thereby I noticed that the previous (non propagated) set was not fulfilling the assumption that it’s a convex polygon with vertices ordered in counter-clockwise direction (premise). A convex polygon should have exactly one extrema in any direction, so including the direction along the negative y-axis. As we can see below, this wasn’t the case, -0.025 and -0.325 are two different extremas (or at least whatever other extrema was following -0.325, sorry [1] is closed in image).

Wrong assumption

I wrote the following hacky function to test the premise

bool has_more_than_one_local_minimum_in_y_axis_direction(const shared_ptr<ReachPolygon> &polygon) {
  auto polygon_vertices = polygon->vertices();

  bool reached_minimum = false;
  bool decreasing = false;
  for (int i = 1; i < polygon_vertices.size() + 2; i++) {
    auto &cur = polygon_vertices[i % polygon_vertices.size()];
    auto &prev = polygon_vertices[(i-1) % polygon_vertices.size()];

    // Increased, we have reached a minimum
    if (decreasing && cur.y > prev.y) {
      if (reached_minimum) {
        // Multiple minimums
        return true;
      }

      reached_minimum = true;
      decreasing = false;
    }

    if (cur.y < prev.y) {
      decreasing = true;
    }
  }

  return false;
}

I would spare you the entire journey here, but I chased this down to reach::create_zero_state_polygon(double const& dt = 0.1, double const& a_min = -6, double const& a_max = 6), which will compute a non-convex polygon with the given parameters. More specifically, intersecting

[
    (0.03, 0.6),
    (0, 0.3),  # r
    (-0.03, -0.3),  # OUTSIDE
    (-0.03, -0.6),  # q
    (0, -0.3),
    (0.03, 0.3),
]

with

a = -1.2
b = 0
c = 0.036

would result in

[
    (-0.03, -0.6),
    (0, -0.3),
    (0.03, 0.3),
    (0.03, 0.6),
    (0, 0.3),
    # NEW
    (-0.03, -0.3),
    (-0.03, -0.208),  # problematic
]

This is because the edge between the following points is essentially on the edge of the half-space.

(-0.03, -0.3),  # OUTSIDE
(-0.03, -0.6)

@Edmond, @gerald_w, do you have any input on this? I’m not sure whether this change would make other assumptions wrong and thus have unexpected consequences.

On another note, why is reach::create_zero_state_polygon convexifying the resulting polygon? Shouldn’t it be readily convex if we started with a convex polygon and only intersected it with a bunch of half-spaces. And why is ReachPolygon::sort_vertices_bottom_left_first convexifying it again?

Hi Boyan,

thanks for this detailed post! I took a look at the intersect_halfspace() method in detail and, indeed, what you described could lead to problems. For some reason, using the example vertices and halfspace values you provided did not lead to a wrong result in my case. However, I noticed, that the line to check whether a point lies inside a halfspace should be:

bool ret = (a * vec_vertices[i].x + b * vec_vertices[i].y <= c)

Note the <= instead of strict inequality < as in the current code, which then complies with a halfspace equation of the form ax + by <= c.

Nevertheless, I think numerical issues could occur and still lead to to wrong results as you described. For this case, adding a numerical tolerance value would probably be a good idea. Unfortunately, I couldn’t reproduce the problematic vertex which you described. We will fix these two points (changing the inequality sign and adding numerical tolerance value) in the next version. Could you let me know whether changing the sign to <= fixes the problem you described in your case?

Best,
Gerald

Hi Gerald,

thanks for the reply and further investigation. I though I had issues with just <= c, but am now unable to reproduce them. I inserted the premise again as below and was able to reproduce the issue right away for < c.

I then went ahead and placed the premise directly at the beginning and end of the ReachPolygon::intersect_halfspace method and tested for <= c and < c + epsilon. It was failing only at the end, but very occasionally, and for other intersections not involved in the creation of the zero state polygon. You can see some examples below. While doing that, I finally realized it’s of course not a problem that some vertices are very close to the intersecting half-space. It’s when an edge is essentially in the same direction that things go bad, and I’m not really handling this case at all with the epsilon. I did try checking when the determinant detA is small and handling it this way, but I can’t remember why I dropped the approach :grin:. I’ll give it a try again.

Failing premise for <= c

Failing premise for < c + 1e-10

Failing premise for < c + 1e-3

So this is the situation I imagine is problematic

The wider the polygon is, the harder it is computationally to find the intersection between q->qq and the half-plane, as the lines will be in nearly the same direction. If the result is slightly off downwards, we get a “dip” in the polygon making it non-convex. If I understand the algorithm correctly, then in such cases the determinant should be pretty much zero. I tried checking that and just omitting the vertex resulting from that “hard” intersection (here X). That still didn’t resolve the issue, I am often getting vertices that are very similar to q and r, and they are occasionally making the premise fail.

I don’t know how important it is to really get to the bottom of this. With <= c, I am at least not seeing any weird behavior.

I think I finally did get to the bottom of it, or at least this particular premise is no longer failing on my scenarios.

Imagine now r (wlog, same for q) is almost at the intersecting half-space. Due to numerical issues, the computed intersection can be slightly off, resulting in a new local minima and breaking my premise, as in the picture below.

But in such cases, we don’t really need to compute an intersection at all, we might just relatively safely drop yet another vertex. With that, my intersection implementation becomes:

auto intersection = [&a, &b, &c](Vertex& pt1, Vertex& pt2) {
    // WARNING: This heavily relies on the fact that pt2
    // is qq/rr, i.e. that this is the point outside the half-space.

    // Yet again, epsilon is arbitrary and perhaps
    // a cleaner solution is possible. This is just POC
    if (a*pt1.x + b*pt1.y > c - 1e-5) {
      // i.e. q/r is essentially on the intersecting half-space
      return std::optional<Vertex>();
    }
  
    double a21 = pt2.y - pt1.y;
    double a22 = -(pt2.x - pt1.x);
    double b2 = a21 * pt2.x + a22 * pt2.y;
    double detA = a * a22 - b * a21;

    // See previous (forum!) comment. Epsilon is arbitrary...
    if (std::abs(detA) <= 1e-10) {
        return std::optional<Vertex>();
    }

    return std::optional<Vertex>(Vertex{(c * a22 - b2 * b) / detA, (a * b2 - a21 * c) / detA});
};

It’s all nice and formal now, but was it really worth it? :grin:

P.S.: And yes, I finally left it with just <= c, I think that’s sufficient.

1 Like

Hi Boyan,

thanks for the further detailed investigation. We fixed the <=c part in the code.

1 Like

Hi Gerald,

I’m happy to hear that.

Sorry, I’m probably being very annoying, but do you have any input on the two extra conditions I added in my last post?

I realise you would be hesitant to add them in your code without any formal proof. Just wondering whether to leave them in my own fork.