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

Exposed planners from latest ompl release. #338

Merged
merged 4 commits into from
Nov 11, 2016

Conversation

rbbg
Copy link
Contributor

@rbbg rbbg commented Nov 1, 2016

Description

This PR exposes the following OMPL planners:

geometric::FMT
geometric::BFMT
geometric::PDST
geometric::STRIDE
geometric::BiTRRT
geometric::LBTRRT
geometric::BiEST
geometric::ProjEST
geometric::LazyPRM
geometric::LazyPRMstar
geometric::SPARS
geometric::SPARStwo

Several other planners are available in OMPL, but from I found those to not work out of the box so some work would be required in integrating those.

To test: I have a forked moveit_resources (over here) that has a ompl_planning.yaml with (parameter-less) configurations for these planners.

Due to the difference in OMPL version between kinetic and jade/indigo, this should not be cherry picked (I think).

@davetcoleman
Copy link
Member

Awesome, thank you!

Can you also add default parameter values for these planners in the Setup Assistant in this PR so that its easier for new users to configure these planners?

@rbbg
Copy link
Contributor Author

rbbg commented Nov 1, 2016

I would like to, but I honestly have no clue what would be logical settings for most of these parameters, so I don't really know how to go about it. I figured it was perhaps best let to OMPL to select the defaults. Thoughts on how to go about this?

@davetcoleman
Copy link
Member

In the Setup Assistant defaults, just copy the default already provided in each planner's config. e.g. for FMT you can see its defaults in the constructor

This allows users to easily tweak the settings in the future, and keeps the planner setup consistent in MoveIt!

@rbbg
Copy link
Contributor Author

rbbg commented Nov 4, 2016

I'm working on this but am running into a couple of problems so it might be a couple of days.

Removed VFRRT.

Fixed BFMT typo.

Removed problematic planners.

Removed LazyLBTRRT due to error with InfSampler.
@rbbg
Copy link
Contributor Author

rbbg commented Nov 10, 2016

For some reason FMT and BFMT are not finding any solutions at the moment, so I need to see where that is coming from, I pretty sure they were working before so not sure what that is about. Also, my clang-format plugin seems to have some problems :(

@davetcoleman
Copy link
Member

For some reason FMT and BFMT are not finding any solutions at the moment, so I need to see where that is coming from

Is a parameter set incorrectly in theses defaults? I'm ok merging these new features now and worrying about tweaking individual parameters later.

Also, my clang-format plugin seems to have some problems :(

I'm curious - did you have to manually fix the formatting then? What version are you using?

@rbbg
Copy link
Contributor Author

rbbg commented Nov 11, 2016

Is a parameter set incorrectly in theses defaults? I'm ok merging these new features now and worrying about tweaking individual parameters later.

I don't think it's a parameter. I tried with an empty configuration but that didn't work either. I'll try to take a better look at it as soon as possible.

I'm curious - did you have to manually fix the formatting then? What version are you using?

I was using clang-format-3.8 but that isn't compatible as demonstrated by #344. As a matter of fact, the formatting changes I saw in the diff of that PR were identical to the ones I was having in my moveit_config_data.cpp file. Switching to clang-format-3.6 fixed it.

@davetcoleman davetcoleman merged commit 26459b0 into moveit:kinetic-devel Nov 11, 2016
@davetcoleman
Copy link
Member

Thanks so much! Please open another PR when you get a chance to look at FMT / BFMT

@gavanderhoorn
Copy link
Contributor

@rbbg: is the 'regular' binary distributed OMPL release (on Kinetic) enough for this, or do we need a source install?

@davetcoleman
Copy link
Member

@gavanderhoorn the binary should work because Travis passed that uses OMPL from binary

@gavanderhoorn
Copy link
Contributor

Right. Missed that. Thanks.

@rbbg rbbg deleted the kinetic-ompl-planners branch November 11, 2016 12:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants