gerryai / pddl-parser

Java parser for the Planning Domain Definition Language (PDDL)
GNU General Public License v3.0
11 stars 4 forks source link

Adds support to numeric-fluents and actions-costs. #2

Closed bymarkone closed 8 years ago

purple52 commented 8 years ago

This is great - thanks for this! It's a big pull request, so I need to spend some time reading through it and digesting the changes. But I will take a proper look as soon as I can.

Are you using the parser on a project?

bymarkone commented 8 years ago

I did have to build a planner for a discipline in master classes. So it is kind of a project. Nothing big though. Thanks for looking at the pool request, it was pretty fun implementing it. I could have been far more diligente with code, but I had a really short time to finish all. Let me know if there is something to fix or upgrade, I will need some more fun for vacation time. :)

purple52 commented 8 years ago

Sorry for the delay. I've finally taken a proper look though. You've been very thorough. It's good to see that the code was extensible enough to support this, so thanks for spending the time to figure out how to use my approach.

I'm going to merge this as-is, but there is one point I think needs addressing at some stage: how we handle :numeric-fluents vs :action-costs requirements. I based the Antlr grammar on this: http://www.plg.inf.uc3m.es/ipc2011-deterministic/attachments/OtherContributions/kovacs-pddl-3.1-2011.pdf which says that the :action-costs requirement is a limited form of :numeric-fluents. So I think that metricsDef should be guarded by :numeric-fluents instead of :action-costs (in fact, this is shown in the commented line [<metric-spec>]:numeric-fluents (https://github.com/gerryai/pddl-parser/blob/master/src/main/antlr4/PDDL31.g4#L198). However, I can see that your current definition of metricsDef only supports one type of metric definition currently so maybe we need to extend that to support the full range before making changes.

Anyway, I don't have a firm view of how to implement the :action-costs limitations in the grammar - they might need to be enforced in the parser code instead.

I would love to see your planner when you've finished. And if you hit any practical coding issues using the domain/problem model that the parser outputs, please let me know.