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

moveit_py: Add Policy Class #2494

Merged
merged 1 commit into from
Dec 6, 2023
Merged

Conversation

peterdavidfagan
Copy link
Member

@peterdavidfagan peterdavidfagan commented Oct 27, 2023

Description

This pull request begins to add tooling to moveit_py that will enable the deployment of learnt policies using the moveit_servo package.

Initial example of API usage is being developed here.

Related feature request to help enable registering sensors and moveit_servo interface from config: here

@codecov
Copy link

codecov bot commented Oct 27, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (73f4551) 50.40% compared to head (662b8ed) 50.81%.
Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2494      +/-   ##
==========================================
+ Coverage   50.40%   50.81%   +0.42%     
==========================================
  Files         391      392       +1     
  Lines       31982    32154     +172     
==========================================
+ Hits        16117    16336     +219     
+ Misses      15865    15818      -47     

see 7 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@AndyZe
Copy link
Member

AndyZe commented Oct 27, 2023

Now this sounds interesting

@peterdavidfagan
Copy link
Member Author

peterdavidfagan commented Oct 30, 2023

Here is an example of specifying random pose commands using the current abstract base class:

IMG_2467.MOV

Example Script

One item I am considering revising is how parameters are being read. Currently sensor topic subscriptions and their synchronisation settings are read from a parameters config specified by the generate_parameter_library. One thing that is evident is that these parameters do not need to be exposed as a node parameter as they are solely used for initial node configuration (and hence they are implicitly represented through the nodes subscribers and publishers). I regularly use hydra to manage ML experiment configs, I believe it may be easier to leverage such a library for managing policy deployment configs within the Python library.

Copy link
Contributor

@sea-bass sea-bass left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is great -- love the video and the application of the new Pose command for servo!

I have nothing of value to add to this PR review.

The only major thing, which I'm sure you already have planned, is to include your example usage in the moveit2_tutorials repo.

moveit_py/moveit/policies/policy.py Outdated Show resolved Hide resolved
moveit_py/moveit/policies/policy.py Outdated Show resolved Hide resolved
moveit_py/moveit/policies/policy.py Outdated Show resolved Hide resolved
moveit_py/moveit/policies/policy.py Outdated Show resolved Hide resolved
moveit_py/moveit/policies/policy.py Outdated Show resolved Hide resolved
moveit_py/moveit/policies/policy.py Outdated Show resolved Hide resolved
moveit_py/moveit/policies/policy.py Outdated Show resolved Hide resolved
update command interfaces

Update moveit_py/moveit/policies/policy.py

Co-authored-by: Sebastian Castro <[email protected]>

Update moveit_py/moveit/policies/policy.py

Co-authored-by: Sebastian Castro <[email protected]>

Update moveit_py/moveit/policies/policy.py

Co-authored-by: Sebastian Castro <[email protected]>

Update moveit_py/moveit/policies/policy.py

Co-authored-by: Sebastian Castro <[email protected]>

Update moveit_py/moveit/policies/policy.py

Co-authored-by: Sebastian Castro <[email protected]>

Update moveit_py/moveit/policies/policy.py

Co-authored-by: Sebastian Castro <[email protected]>

update script description
@peterdavidfagan
Copy link
Member Author

peterdavidfagan commented Oct 30, 2023

Thanks @AndyZe and @sea-bass,

The only major thing, which I'm sure you already have planned, is to include your example usage in the moveit2_tutorials repo.

I will look to add tutorial examples for each of the command interfaces. I plan to iterate on the existing API for a week or so (currently using it for model deployment for one of my projects). Once the API has matured a bit I will be sure to provide tutorial examples, looking forward to potentially seeing more of the robot learning research community use this API.

@peterdavidfagan peterdavidfagan changed the title WIP: MoveItPy: Add Policy Class MoveItPy: Add Policy Class Oct 30, 2023
@peterdavidfagan peterdavidfagan changed the title MoveItPy: Add Policy Class moveit_py: Add Policy Class Oct 30, 2023
@sjahr
Copy link
Contributor

sjahr commented Dec 6, 2023

@peterdavidfagan Can we merge this?

@peterdavidfagan
Copy link
Member Author

Hi @sjahr,

This can be merged.

Note: I am actively developing this API and examples of baseline models deployed with the API. I am happy to open another PR with changes and/or fixes in the future. As this will be another few weeks, more than happy for this to be merged in the meantime.

@AndyZe AndyZe merged commit 1d2bf44 into moveit:main Dec 6, 2023
12 checks passed
@sjahr
Copy link
Contributor

sjahr commented Dec 6, 2023

Awesome, thank you @peterdavidfagan

@sea-bass
Copy link
Contributor

sea-bass commented Dec 7, 2023

This PR broke the docs build in the moveit2_tutorials repo BTW.

Extension error (sphinxcontrib.doxylink.doxylink):
Handler <function setup_doxylink_roles at 0x7fa6fe874d30> for event 'builder-inited' threw an exception (exception: Cannot add override to non-function 'moveit::policies::policy::Policy::activate_policy')

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.

4 participants