swri-robotics / descartes_light Goto Github PK

Currently the Vertex class (used for bundled properties in the BGL search methods) holds a subset of the information needed by the search methods (distance and color). Other properties, namely the vertex predecessor, is being managed outside the graph.

For clarity, it seems like the graph should either:

1. manage all the vertex properties required for the search in the graph
   • Pros:
     • Bundled properties are somewhat easier to manage in the graphs
   • Cons:
     • Some fields of the Vertex class need to be populated by the solver build method and should be held constant during the search (sample and rung_idx), and other methods will be filled out by the search method (color, distance, etc.). This is somewhat confusing and prone to error
2. all of those properties should be managed outside of the search.
   • Pros:
     • The Vertex class can be composed of only "read-only" members that get populated by the solver build method, which would reduce ambiguity about the class
   • Cons:
     • The containers for the vertex properties must be stored/maintained inside the solver class
     • The property containers would need to be passed to the graph serialization functions

After the removal of the KinematicsInterface and CollisionInterface in #42, some if not all utilities in descartes_light/utils.h may not be relevant any more. If we don't need them or plan on using them, they should be removed

• Add README
• Document API

descartes_light exports the C++ version to use in such a way that the compilation of tesseract_planning fails. Descartes enforces C++ 14, tesseract_planning needs C++ 17.

As per title.

Would perhaps be nice to add a sentence or two to the readme?

Hi!
I'm running into some segfault bugs, where if I attempt to use more than 1 thread, I get pure virtual method called. This is true for all solvers. Do I need to provide an implementation of handling threads?
Curious if this is a known issue?

The current DAGSearch returns an infinite cost and a vector of zeros for the path when no edges are present in the graph. This confusing because the all-zero-index path is actually a valid path, but the search actually failed. We should change the API to throw an exception when a solution cannot be correctly generated

Add support for CI

This can quickly become an issue when you have more than a few free degrees of freedom. A quick way to address this, if you cannot limit the search space of the free degree of freedom is increase your swap space. This can easily be done now on Ubuntu 18.04 and newer shown here.

Currently sampler classes have to be constructed with the tool pose that they are going to sample. It seems unnecessary to have to create a new sampler object (with possible many input arguments) for every pose that you want to sample. The sampling of the pose should be independent of the pose itself. We should change the sampler interface sample method to take as input the pose to be sampled. Then you could use the same single sampler for any number of poses

bool sample(const Eigen::Transform<FloatType, 3, Eigen::Isometry>& tool_pose, std::vector<FloatType>& solution_set)

PRs #76, #77 instantiate some unnecessary solver permutations, like a dynamic edge-adding Dijkstra full search, that are useful for benchmarking but would not be selected for real problems. We should find a more succinct construction pattern that can construct these solvers when needed without manually defining them.

@mpowelson and @marip8 This is likely to cause issues when releasing. Propose the following renaming. It is possible that they still may require prefixing it with swri but I will ask for the following first then go from there. Let me know your thoughts?

Repository descartes_light -> ladder_graph_planner
package descartes_light -> ladder_graph_solver
package descartes_samplers -> ladder_graph_samplers

The cost_recorder event visitor makes a copy of the edge weight property map every time it is utilized in order to get the cost of an edge. It seems like there should be a more efficient way of getting the edge weight that avoids this unnecessary amount of copying

Also, the class should probably be renamed to distance_recorder since it is actually recording the distance from the start vertex to each visited vertex

From MRip: It probably makes sense for us to save the results of the benchmark into a structured format (like CSV, YAML, JSON, XML, etc.), rather than parsing the text that we arbitrarily wrote to document the benchmark results.

My vote would be to use YAML in a structure like this:

- name: planner_name
  results:
    - n_waypoints: 10
      n_samples: 10
      dof: 6
      build_time: 0.05185
      search_time: 6.2715678
    - n_waypoints: 10
      n_samples: 50
      dof: 6
      build_time: 0.06768
      search_time: 8.123486

@Levi-Armstrong any thoughts on the structure of the benchmark output, especially if we plan to use Google Benchmark in the future?

Originally posted by @marip8 in #78 (comment)

Add a representation, samplers, and evaluators for 6-DoF Cartesian paths such that the solvers in this repository can be used for tool path operations like smoothing

Was this intentional or should this be modified to work in the case where the start position is more than 2pi from +-pi?

This would allow for metadata to be stored about the sampled state which can be used during edge evaluation. For example, I have a series of cartesian poses that can be rotated about the z-axis 0 or 180deg, but only 0's can be connected to each other and 180's to each other. This can not easily be recomputed when the path is along a highly contoured surface. @marip8 what do you think?

template <typename FloatType>
struct State
{
Eigen::Matrix<FloatType, Eigen::Dynamic, 1> state;
std::shared_ptr<void> user_data;
}

It looks like this is fixed in opw_kinematics using the code below. However, we may want to instead use the method described here

if ( NOT CMAKE_SYSTEM_NAME2 MATCHES "aarch64" AND NOT CMAKE_SYSTEM_NAME2 MATCHES "armv7l" AND NOT CMAKE_SYSTEM_NAME2 MATCHES "unknown" )
  set(OPW_COMPILE_OPTIONS_PUBLIC -mno-avx)
endif()

Descartes has a tendency to run out of memory. It may possible to leverage Boost::Interprocess Memory mapped file. This link shows an example for shared memory but may be similar implementation for mapped file.

Hello,

This ROS-I presentation mentions the possibility of relying on RRT or OMPL planners to solve the ladder search problem. Is support for such functionalities being worked on?

I am trying to incorporate this package into a ros2 workspace. Would someone be able to supply an example of a ros2 package successfully using this project? I can't seem to get it linked properly.

The issue is that core creates the target if one does not exist but it must do the same in the packages config.cmake so it can be used by other packages. A extras.cmake needs to be added to this package with does this.