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

Grasp Generation in PDDLStream planning #53

Merged
merged 20 commits into from
Jan 9, 2023
Merged

Conversation

sea-bass
Copy link
Owner

@sea-bass sea-bass commented Jan 6, 2023

It's finally here!

Closes #45

@sea-bass sea-bass requested a review from AndyZe January 6, 2023 14:49
@sea-bass sea-bass changed the base branch from main to grasp-gen-pick-object January 6, 2023 14:49
Comment on lines 160 to 164
# If a grasp pose is specified, add that instead
if act.type == "pick" and len(act_pddl.args) > 5:
arg = act_pddl.args[5]
if isinstance(arg, Grasp):
act.pose = arg.origin
Copy link
Owner Author

Choose a reason for hiding this comment

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

I deeply dislike these rules. If you have any thoughts on how to better convert an arbitrary PDDLStream plan to an actual action plan, I'd be interested in hearing it.

Even if we move the PDDLStream mappings/primitives to a user workspace, users will also need to touch this code if they're doing anything "non-standard" per our examples.

Copy link
Collaborator

Choose a reason for hiding this comment

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

probably functional programming would help, i.e. the user can pass a list of functions to pddlstream_solution_to_plan(). Most of those functions will probably come from the pyrobosim module but sometimes it might come from the user's custom module.

I'm not an expert on that at all, especially not in Python

Copy link
Owner Author

Choose a reason for hiding this comment

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

That's very good! You can just pass a dictionary mapping from the action name to a function that makes the action itself.

In fact, that's exactly how PDDLStream does the mappings, as seen in the mappings.py file

@sea-bass sea-bass marked this pull request as ready for review January 7, 2023 12:31
Copy link
Collaborator

@AndyZe AndyZe left a comment

Choose a reason for hiding this comment

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

Hmm, first shot at testing this:

python3 examples/demo_pddl.py --example 01_simple --verbose --> Works fine

python3 examples/demo_pddl.py --example 05_nav_grasp_stream --verbose -->

> FileNotFoundError: [Errno 2] No such file or directory: '/home/andy/.local/lib/python3.10/site-packages/pyrobosim/utils/../data/pddlstream/domains/05_nav_grasp_stream/domain.pddl'

Resolved: I hadn't checked out the right branch, and also needed to run pip install ./pyrobosim

@sea-bass
Copy link
Owner Author

sea-bass commented Jan 7, 2023

Hmm, first shot at testing this:

python3 examples/demo_pddl.py --example 01_simple --verbose --> Works fine

python3 examples/demo_pddl.py --example 05_nav_grasp_stream --verbose -->

FileNotFoundError: [Errno 2] No such file or directory: '/home/andy/.local/lib/python3.10/site-packages/pyrobosim/utils/../data/pddlstream/domains/05_nav_grasp_stream/domain.pddl'

I think that may just be that a colcon build and/or pip install is needed?

@sea-bass sea-bass changed the base branch from grasp-gen-pick-object to main January 8, 2023 00:24
docs/source/usage/tamp.rst Outdated Show resolved Hide resolved
docs/source/usage/tamp.rst Outdated Show resolved Hide resolved
; ISCOLLISIONFREE: Checks that an object pose at a particular location is
; collision free with all other objects at that location.
(:derived (IsCollisionFree ?l ?o ?p)
(not (exists (?obs ?pobs)
Copy link
Collaborator

Choose a reason for hiding this comment

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

also don't see obs and pobs defined above

Copy link
Owner Author

Choose a reason for hiding this comment

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

The exists predicate is kind of like a lambda, in that you define new variables that only exist in that scope.

Copy link
Collaborator

Choose a reason for hiding this comment

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

is that what makes it a "derived" predicate?

Copy link
Owner Author

Choose a reason for hiding this comment

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

No, but both derived predicates and things like exists are syntactic sugar for PDDL.

A derived predicate just defines a shorthand for complex logic with existing predicates to write done as a single predicate. Think of it like a "function".

exists is similarly some common logical expression you can use in place of a giant string of ANDs and ORs. There are others like imply, forall, etc. as shown here: https://planning.wiki/ref/pddl/domain#exists

Comment on lines +136 to +144
(and (or (Obj ?entity) (Robot ?entity))
(Location ?loc)
(or (At ?entity ?loc)
(exists (?s)
(and (Room ?loc) (Location ?s)
(At ?entity ?s) (AtRoom ?s ?loc))
)
)
)
Copy link
Collaborator

Choose a reason for hiding this comment

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

?s isn't defined above. Is it a sampled location?

Also, is this reading of the logic correct:

  • entity is an object type OR robot type
  • AND ?loc is a location type
  • entity is at ?loc OR
    • ?s exists AND:
      • ?loc is a room
      • AND ?s is a location
      • AND ?entity is at ?s
      • AND ?s is at room ?loc

Pretty complicated! In other words, the sampled location and ?loc are the same and ?entity is there?

Copy link
Owner Author

Choose a reason for hiding this comment

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

The exists predicate is kind of like a lambda, in that you define new variables that only exist in that scope.

And yep, your reading of that logic is exactly right. Those Has and HasAll derived predicates are pretty tough to get working, which is why PDDL domain definition is kind of an art and people want to get away from it.

@sea-bass sea-bass requested a review from AndyZe January 8, 2023 13:07
@sea-bass sea-bass merged commit dba7efc into main Jan 9, 2023
@sea-bass sea-bass deleted the pddlstream-grasp-gen branch January 9, 2023 12:23
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.

Add basic grasp generation pipeline
2 participants