aig-upf / tarski

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

Bug parsing real constant #114

Closed alvalentini closed 3 years ago

alvalentini commented 3 years ago

I got an error running the following code:

reader = tarski.io.PDDLReader(raise_on_error=True, strict_with_requirements=False)
reader.parse_domain('domain.pddl')
problem = reader.parse_instance('problem.pddl')

Input: pddl files

Possible patch:

diff --git a/src/tarski/io/_fstrips/common.py b/src/tarski/io/_fstrips/common.py
index dec1730..3e6485b 100644
--- a/src/tarski/io/_fstrips/common.py
+++ b/src/tarski/io/_fstrips/common.py
@@ -67,7 +67,7 @@ def get_requirements_string(problem):
 def create_number_type(lang):
     """ Creates a sort corresponding to the PDDL "number" """
     # For the moment being, we assume that PDDL "number"s are unbound integers
-    parent = lang.get_sort("Integer")
+    parent = lang.get_sort("Real")
     lower = parent.lower_bound
     upper = parent.upper_bound
     lang.interval("number", parent, lower, upper)
miquelramirez commented 3 years ago

Thanks @alvalentini for reminding us of that comment "For the moment being..."

@gfrances I remember we had decided to interpret number as an integer to avoid breaking something... can you remember what was that? If you can't, then it is definitely time to move on and respect the PDDL 2.1 specification.

gfrances commented 3 years ago

Hi @alvalentini,

I'm trying to reproduce some error by running the code you provide on your domain and instance, but I'm not getting there. See e.g. commit 4c263b839. At the moment the parser is complaining that the :goal section is after the :init section, which I presume is not the error you were getting. Could you report exactly what error you get, and with what domain/instance files that happens?

alvalentini commented 3 years ago

The error I get parsing the domain I provided is the following:

Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/home/alvalentini/tarski/src/tarski/io/fstrips.py", line 58, in parse_domain
    self.parse_file(filename, 'domain')
  File "/home/alvalentini/tarski/src/tarski/io/fstrips.py", line 55, in parse_file
    self.parser.visit(domain_parse_tree)
  File "/home/alvalentini/.local/lib/python3.8/site-packages/antlr4/tree/Tree.py", line 34, in visit
    return tree.accept(self)
  File "/home/alvalentini/tarski/src/tarski/io/_fstrips/parser/parser.py", line 803, in accept
    return visitor.visitDomain(self)
  File "/home/alvalentini/tarski/src/tarski/io/_fstrips/parser/visitor.py", line 19, in visitDomain
    return self.visitChildren(ctx)
  File "/home/alvalentini/.local/lib/python3.8/site-packages/antlr4/tree/Tree.py", line 44, in visitChildren
    childResult = c.accept(self)
  File "/home/alvalentini/tarski/src/tarski/io/_fstrips/parser/parser.py", line 2621, in accept
    return visitor.visitStructureDef(self)
  File "/home/alvalentini/tarski/src/tarski/io/_fstrips/parser/visitor.py", line 169, in visitStructureDef
    return self.visitChildren(ctx)
  File "/home/alvalentini/.local/lib/python3.8/site-packages/antlr4/tree/Tree.py", line 44, in visitChildren
    childResult = c.accept(self)
  File "/home/alvalentini/tarski/src/tarski/io/_fstrips/parser/parser.py", line 2703, in accept
    return visitor.visitActionDef(self)
  File "/home/alvalentini/tarski/src/tarski/io/_fstrips/reader.py", line 187, in visitActionDef
    precondition, effects = self.visit(ctx.actionDefBody())
  File "/home/alvalentini/.local/lib/python3.8/site-packages/antlr4/tree/Tree.py", line 34, in visit
    return tree.accept(self)
  File "/home/alvalentini/tarski/src/tarski/io/_fstrips/parser/parser.py", line 3070, in accept
    return visitor.visitActionDefBody(self)
  File "/home/alvalentini/tarski/src/tarski/io/_fstrips/reader.py", line 196, in visitActionDefBody
    effs = self.visit(ctx.effect())
  File "/home/alvalentini/.local/lib/python3.8/site-packages/antlr4/tree/Tree.py", line 34, in visit
    return tree.accept(self)
  File "/home/alvalentini/tarski/src/tarski/io/_fstrips/parser/parser.py", line 4154, in accept
    return visitor.visitConjunctiveEffectFormula(self)
  File "/home/alvalentini/tarski/src/tarski/io/_fstrips/reader.py", line 318, in visitConjunctiveEffectFormula
    effect = self.visit(sub_ctx)
  File "/home/alvalentini/.local/lib/python3.8/site-packages/antlr4/tree/Tree.py", line 34, in visit
    return tree.accept(self)
  File "/home/alvalentini/tarski/src/tarski/io/_fstrips/parser/parser.py", line 4337, in accept
    return visitor.visitAtomicEffect(self)
  File "/home/alvalentini/tarski/src/tarski/io/_fstrips/reader.py", line 324, in visitAtomicEffect
    return self.visit(ctx.atomic_effect())
  File "/home/alvalentini/.local/lib/python3.8/site-packages/antlr4/tree/Tree.py", line 34, in visit
    return tree.accept(self)
  File "/home/alvalentini/tarski/src/tarski/io/_fstrips/parser/parser.py", line 4500, in accept
    return visitor.visitAssignEffect(self)
  File "/home/alvalentini/tarski/src/tarski/io/_fstrips/reader.py", line 441, in visitAssignEffect
    rhs = self.language.dispatch_operator(get_function_from_symbol(operator), Term, Term, lhs, rhs)
  File "/home/alvalentini/tarski/src/tarski/fol.py", line 383, in dispatch_operator
    return op(lhs, rhs)
  File "/home/alvalentini/tarski/src/tarski/theories.py", line 139, in handler
    lhs, rhs = cast_to_closest_common_numeric_ancestor(lang, lhs, rhs)
  File "/home/alvalentini/tarski/src/tarski/syntax/ops.py", line 25, in cast_to_closest_common_numeric_ancestor
    return lhs, Constant(lhs.sort.cast(rhs), lhs.sort)
  File "/home/alvalentini/tarski/src/tarski/syntax/sorts.py", line 112, in cast
    y = self.encode(x)  # can raise ValueError
  File "/home/alvalentini/tarski/src/tarski/syntax/sorts.py", line 194, in int_encode_fn
    raise ValueError()  # We don't want 1.2 to get encoded as an int
ValueError

The version of tarski is v0.7.0 (commit 7d955e535fbbca012bfd1a12402b97febc6b35b9).

alvalentini commented 3 years ago

The domain and problem you added in commit 4c263b83916c0863dd93cbd96d49e09ff22b69c8 are not the ones I provided in the issue.

gfrances commented 3 years ago

Oops my bad then, must have mixed them with some other pddl files. Will re-check that in a bit!

gfrances commented 3 years ago

@miquelramirez , maybe you can have a look at commit 5bc7980 and let me know what you think. I've just implemented Alessandro's suggested fix, which by default interprets that numbers are real numbers, not integers. I've had to fix one minor test, but otherwise should be good to go.

gfrances commented 3 years ago

This should be fixed in devel now

alvalentini commented 3 years ago

When do you think this fix will be available in a new tarski release?

gfrances commented 3 years ago

We can make a new release easily, but you should be able to use the new feature by pulling with pip directly from Github, at whatever fixed commit ID you prefer. The syntax for this varies slightly whether you're using a requirements.txt file, a setup.py file, or some other python environment manager. See e.g. this for some examples. If that doesn't work, let me know.