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

Bug parsing real constant #114

Closed
alvalentini opened this issue Jul 9, 2021 · 9 comments
Closed

Bug parsing real constant #114

alvalentini opened this issue Jul 9, 2021 · 9 comments
Assignees
Labels
bug Something isn't working

Comments

@alvalentini
Copy link

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
Copy link
Member

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.

@miquelramirez miquelramirez self-assigned this Jul 10, 2021
@miquelramirez miquelramirez added the bug Something isn't working label Jul 10, 2021
@gfrances
Copy link
Member

gfrances commented Jul 12, 2021

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 4c263b8. 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
Copy link
Author

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 7d955e5).

@alvalentini
Copy link
Author

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

@gfrances
Copy link
Member

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

@gfrances
Copy link
Member

@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 added a commit that referenced this issue Jul 13, 2021
Use real numbers by default when parsing with strict_with_requirements=False
@gfrances
Copy link
Member

This should be fixed in devel now

@alvalentini
Copy link
Author

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

@gfrances
Copy link
Member

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants