aig-upf / tarski

Tarski - An AI Planning Modeling Framework
Apache License 2.0
59 stars 20 forks source link

Implement Writers and Readers #5

Closed gfrances closed 4 years ago

gfrances commented 6 years ago

We would like to implement writers and readers in two different formats: json and PDDL. This will definitely help adjust and improve the current code design / API. The code should live under the tarski.io module. Not sure if we want to reuse code from the FS planner and/or FD's python parser, that might be a reasonable first step - what do you think, @miquelramirez ?

miquelramirez commented 6 years ago

I think that reusing the code from the FS planner as much as is convenient is the way to go. FD's Python Parser is not something we want to go back to.

gfrances commented 6 years ago

I think that reusing the code from the FS planner as much as is convenient is the way to go. FD's Python Parser is not something we want to go back to.

Ok, I should try to tackle this next week. I'll start trying to put the PDDL reader in place. Now, what approach would you recommend, miquel? Not sure what you mean bu "resuing the code from FS", since the code from FS is basically using FD's python parser. You mean ditching that and going with an ANTLR-based parser? I would definitely love to have something as properly encapsulated as that, although I'm not yet too familiar with the specifics of ANTLR. Is there any shortcoming if we go that way? I understand that this adds no extra dependency, if we commit the appropriate ANTLR-generated files, right? OTOH, how monolithic is that - i.e., if I want to add a new feature to the language just for some research idea, how much work does that imply, and how modularly can I do that?

miquelramirez commented 6 years ago

Great that you wrote about this, you just reminded I need to add you to a repo I created last night.

I would definitely love to have something as properly encapsulated as that, although I'm not yet too familiar with the specifics of ANTLR. Is there any shortcoming if we go that way?

We could totally reuse the grammar, but we would have to adjust how the parser events are handled, hooking into Tarski rather than the FD AST stuff. That should not be too arduous.

I understand that this adds no extra dependency, if we commit the appropriate ANTLR-generated files, right?

There's the dependency with ANTLR itself, it needs to be installed and it is made in JAVA. That's not awesome, but it is quite contained. You can see that ANTLR creator has made an effort to avoid scaring away people... see his webpage for more info.

$ cd /usr/local/lib
$ wget http://www.antlr.org/download/antlr-4.7.1-complete.jar
$ export CLASSPATH=".:/usr/local/lib/antlr-4.7.1-complete.jar:$CLASSPATH"
$ alias antlr4='java -jar /usr/local/lib/antlr-4.7.1-complete.jar'
$ alias grun='java org.antlr.v4.gui.TestRig'

those are the install instructions for Linux. You need antlr to generate the lexer and parser classes, very much as you do with flex and bison, if you don't change the grammar, the generated code remains static. See the official docs here for a thorought walk through.

OTOH, how monolithic is that - i.e., if I want to add a new feature to the language just for some research idea, how much work does that imply, and how modularly can I do that?

The event handling code comes in one great lump... see the module python/parser/f_pddl_plus/fstrips/fs_task_loader.py. Every method visitXXX is linked to a rule in the grammar: those are the callbacks that ANTLR will call every time a given rule in the grammar is matched. For an easy one see

 def visitDomainName( self, ctx ) :
        self.domain_name = ctx.NAME().getText().lower()

serves the production rule

domainName
    : '(' 'domain' NAME ')'
    ;

The mapping is quite evident, as the non terminals and terminals in the body of the production rule become a token which can be retrieved via methods generated at runtime of the context object. The stuff between single quotes are terminals... note that I prefer to use the character themselves rather than have illegible stuff like

domainName
    : LPAREN DOMAIN_TOKEN NAME RPAREN
    ;

and a massive list of token definitions. I think there aren't many weird things in there, porting it over should be quite straightforward, actually.

Extensibility: I have been extending it quite a bit... I first started with FSTRIPS circa 2015, added to it state constraints, processes, events, optimal control kind of metric functions... It is way more clear and easy than ever it has been for me!

Modularity needs to be thought out, nevertheless. The grammar itself is what it is... you could have different grammar files (and associated tokeniser, lexer and parser) for each fragment of the language. As long as you keep naming conventions consistent, you should be able to reuse the parsing event handler code to a great extent.

miquelramirez commented 6 years ago

I hope the above bootstraps you @gfrances :)

gfrances commented 5 years ago

I have removed the PDDL io modules, as FSTRIPS subsumes the whole of PDDL, and PDDL problems can be read / written using tarski.io.fstrips. As for the json io module that we discussed, at the moment I do not have too much use for it, since I am developing pyfs to pass all the relevant information through the python bindings, hence doing away with the need of parsing json files, etc.

Do you have other uses for that potential json module, Miquel? Otherwise I'd close this issue.

miquelramirez commented 5 years ago

That I am afraid was a bit rash @gfrances, could you please coordinate with @ekeyder and make sure all the functionality has been carried over?

On Fri, 21 Jun 2019, 04:03 Guillem Francès notifications@github.com wrote:

I have removed the PDDL io modules, as FSTRIPS subsumes the whole of PDDL, and PDDL problems can be read / written using tarski.io.fstrips. As for the json io module that we discussed, at the moment I do not have too much use for it, since I am developing pyfs to pass all the relevant information through the python bindings, hence doing away with the need of parsing json files, etc.

Do you have other uses for that potential json module, Miquel? Otherwise I'd close this issue.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/aig-upf/tarski/issues/5?email_source=notifications&email_token=AADMQKOOUWTERVNK6YSSV23P3PA7PA5CNFSM4EQNXI52YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODYGFOFA#issuecomment-504125204, or mute the thread https://github.com/notifications/unsubscribe-auth/AADMQKJYM5AY6CYMKEXQX4LP3PA7PANCNFSM4EQNXI5Q .

gfrances commented 5 years ago

hehe not sure what functionality you meant, I just deleted two empty files! :-)

gfrances commented 5 years ago

Relevant commit is here: 15e2fc0

miquelramirez commented 5 years ago

So somebody did that already :)

On Fri, 21 Jun. 2019, 17:25 Guillem Francès, notifications@github.com wrote:

Relevant commit is here: 15e2fc0 https://github.com/aig-upf/tarski/commit/15e2fc03b5039a9859c818db80c6d055025b936e

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/aig-upf/tarski/issues/5?email_source=notifications&email_token=AADMQKMKQ6ATPXPWZI3UU6DP3R67TA5CNFSM4EQNXI52YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODYHVSNI#issuecomment-504322357, or mute the thread https://github.com/notifications/unsubscribe-auth/AADMQKPK2QXOABJZK65OP3TP3R67TANCNFSM4EQNXI5Q .

gfrances commented 4 years ago

We finally integrated the FS planner with Tarski through a "json" writer, but the json writer is relatively dependent on the hacks and needs of the FS planner, so it lives as an "adapter" in the FS codebase. I don't think there's a need to write a separate, slightly more standard "json" writer here though, as in the mid term we'd like to expose functionality through python bindings. I'll close the issue then.