search

personalrobotics / prpy

Python utilities used by the Personal Robotics Laboratory.
BSD 3-Clause "New" or "Revised" License
62 stars 19 forks source link

Always check end point in VanDerCorputSampleGenerator() #303

Closed psigen closed 8 years ago

psigen commented 8 years ago

As per discussion in #297, it is valuable to explicitly check the goal state of a planning operation.

This PR addresses the change in SnapPlanner by modifying the underlying VanDerCorputSampleGenerator to always return the start and end points. There doesn't seem to be a compelling reason for this not to be the case, as the generated samples are aliased over the collision checking interval in an arbitrary fashion anyway.

mkoval commented 8 years ago

It looks like Travis timed out. I restarted the build.

DavidB-CMU commented 8 years ago

@psigen It seems we've gone from a method that did too many collision checks, to a method that is apparently doing not enough collision checks.

Can you add some test cases please?

The result of using VanDerCorputSampleGenerator and SampleTimeGenerator should be the same, so changes like this should be made and tested with both sequence generators. .

One extra check won't hurt, but how can the robot can be in a configuration at the end of the trajectory that is within the collision-checking resolution, but yet it collides? Doesn't that mean the resolution is wrong?

Couldn't there be such configurations along the middle of the trajectory?

What do you think @cdellin ?

mkoval commented 8 years ago

But how can the robot can be in a configuration at the end of the trajectory that is within the collision-checking resolution, but yet it collides? Doesn't that mean the resolution is wrong?

This is always the case. We assume that the user selects a collision detection resolution is dense enough to ignore collisions between the sampled points. This is generally accomplished by padding the geometry of everything in the environment by the maximum amount of penetration that may result from the discretization error.

The theoretically sound way of setting padding is to computing the maximum workspace motion of any point on the robot caused by changing the robot's configuration by the collision checking resolution. In practice, we arbitrarily choose a value that is "small enough."

This pull request does not change any of that.

Couldn't there be such configurations along the middle of the trajectory?

The end-point of a trajectory is unique because it is expected to become the starting point of future planning queries. Therefore, those queries will fail if the end-point is not feasible. Note that we would have the same problem if we frequently interrupted trajectories during execution, e.g. by dynamically replanning to adapt to a changing environment. That is not something that we are currently exploring, so this pull request is sufficient to fix the immediate issue.

psigen commented 8 years ago

I think #304 is a simpler way to get this behavior. Let's continue discussion there.