Add Experience-driven Random Trees (ERT and ERTConnect) #783
Conversation
freeMemory() seems to be called before planning, thus deleting the loaded experience
|
@ChamzasKonstantinos can you review this? |
|
Yes, I will probably get to it sometime after next week, where I will have some free cycles. |
|
@ChamzasKonstantinos ping? |
|
@mamoll @ChamzasKonstantinos any update on that? |
There was a problem hiding this comment.
Hi Eric, sorry for the late response. Overall it looks pretty good.
Besides the nits the main points I have are the following:
- Try using clang-format to format the code - there seems to be some formatting issues. There is a specification for ompl here
- I would either remove the Note/Suggestions comments or implement them.
- It would be ideal to reduce the duplicate code that exists as much as possible by abstracting it away in functions when possible and making ERTConnect inherit from ERT.
- Finally, I think have a small demo with one of ompl simple environment would help a lot. Maybe the PlanarManipulatorDemo is suitable for this planner.
| to find similar motion plans. The provided experience is used to | ||
| bias the sampling of candidate states, and their connections. | ||
| @par External documentation | ||
| Èric Pairet, Constantinos Chamzas, Yvan Petillot, Lydia Kavraki, |
There was a problem hiding this comment.
Since this is in IEEE library now, full citation could be added
| PathGeometric getExperienceAsPathGeometric() const | ||
| { | ||
| auto path(std::make_shared<PathGeometric>(si_)); | ||
| for (auto &state : experience_->segment) |
| After calling solve() it will return the updated one, if enabled */ | ||
| PathGeometric getExperienceAsPathGeometric() const | ||
| { | ||
| auto path(std::make_shared<PathGeometric>(si_)); |
There was a problem hiding this comment.
No need to make this a pointer since the derefernced pointer is returned.
| state(si->allocState()), | ||
| phase_span(ps) | ||
| { | ||
| segment.resize(ps); |
There was a problem hiding this comment.
This can be written more concisely like this:
for (unsinged int i = 0 ; i < ps; i++)
segment.emplace_back(si->allocState());
| experience differently. | ||
| @par External documentation | ||
| Èric Pairet, Constantinos Chamzas, Yvan Petillot, Lydia Kavraki, | ||
| Path Planning for Manipulation using Experience-driven Random Trees. |
There was a problem hiding this comment.
Same comment as above
|
|
||
| The resolution of the experience has a significant impact on the | ||
| planner's performance. If no experience is defined, the planner | ||
| exploits a straight path from start to goal. */ |
There was a problem hiding this comment.
Maybe this line should be above the solve documentations
| ompl::base::PlannerStatus ompl::geometric::ERT::solve(const base::PlannerTerminationCondition &ptc) | ||
| { | ||
| checkValidity(); | ||
| auto *goal_s = dynamic_cast<base::GoalSampleableRegion *>(pdef_->getGoal().get()); |
| { | ||
| OMPL_INFORM("%s: Updated experience solves the current problem defition!", getName().c_str()); | ||
|
|
||
| auto path(std::make_shared<PathGeometric>(si_)); |
There was a problem hiding this comment.
You already have a function to do this, called getMotionAsGeometricPath I think
| #include "ompl/base/goals/GoalSampleableRegion.h" | ||
| #include "ompl/tools/config/SelfConfig.h" | ||
|
|
||
| ompl::geometric::ERTConnect::ERTConnect(const base::SpaceInformationPtr &si) |
There was a problem hiding this comment.
If this can inherit from ERT or include it to reduce duplicate code it would be ideal, otherwise same comments for ERT apply to ERTConnect
| @@ -0,0 +1,99 @@ | ||
| /********************************************************************* |
There was a problem hiding this comment.
Is this new PlannerData struct really necessary? From my understanding you are using this to add motions with more than 2 states to the planner data. But you could simply convert the bigger motions to smaller ones (that only have 2 states) and avoid creating a new struct.
|
Hi @ChamzasKonstantinos, thanks for the suggestions. I'll go through the comments, but it might take me a while as I'm currently moving. I'll keep you posted. |
|
Hi, we would like to utilize this planner in our robot and we already use OMPL. Is there any timeline on when it will get merged? Thanks |
|
I have not had the time to address the suggestions. However, the code in the PR is fully functional. |
|
@ericpairet @ChamzasKonstantinos |
|
I ported the ertconnect implementation of this PR into my software where some bug fixes are made as below Also, if your target application is articulated robot (e.g, fetch, pr2, panda) then, using plainmp may be come in handy |
|
@ericpairet , do you have time to look at @HiroIshida's improvements, and potentially incorporate them into your PR (i.e., cherry-pick his commits into your PR)? Or should we consider this PR abandoned? |
This PR contains the Experience-driven Random Trees presented in our recent work on experience-based path planning.
Publication (https://arxiv.org/abs/2103.00448):
Èric Pairet, Constantinos Chamzas, Yvan Petillot, Lydia Kavraki, Path Planning for Manipulation using Experience-driven Random Trees, IEEE Robotics and Automation Letters (RA-L) - IEEE International Conference on Robotics and Automation (ICRA), 2021.