Issue292

Title translator complains about something about an existential condition
Priority bug Status resolved
Superseder Nosy List erez, gabi, malte, rpgoldman
Assigned To malte Keywords
Optional summary

Created on 2011-11-04.20:58:06 by rpgoldman, last changed by malte.

Files
File name Uploaded Type Edit Remove
bams.pddl rpgoldman, 2011-11-04.20:58:38 application/octet-stream
demo01.pddl rpgoldman, 2011-11-04.20:58:24 application/octet-stream
domain-issue292.pddl malte, 2011-11-07.19:25:24 application/octet-stream
problem-issue292.pddl malte, 2011-11-07.19:30:02 application/octet-stream
Messages
msg1937 (view) Author: malte Date: 2011-11-14.16:03:19
Fixed -- I hope. At least the minimal example works now, and the BAMS example,
after commenting out the negated initial fact, now proceeds up to the
"Completing instantiation" step, where it gets stuck.

I've opened a separate issue for this getting stuck bit, which looks like an
inefficient algorithm rather than a bug: issue296. Anyone interested in that
issue, please nosy yourself there.

Gabi: I've added you to the nosy list because you recently worked on the
handling of existential quantification in the normalization code, and my changes
here go in a similar direction. There is nothing you need to do here, but you
might be interested in checking out what I did because it changes the promises
of our normalization code somewhat.

Put briefly, we now completely get rid of existential quantifiers everywhere
(except possibly the goal -- didn't check what happens there), but to be able to
do that, we now need have kinds of parameters in actions and axioms: ones that
are "externally visible" (for actions: actual parameters that should be part of
the grounded action name in the generated plan) and ones that are not.

$ hg diff -c 077dc51dc40f
or (if meld is set up)
$ hg meld -c 077dc51dc40f
will show the relevant changes.
msg1889 (view) Author: malte Date: 2011-11-07.22:08:40
> If I understand ensure_conjunction_sat, it expects all of its parts parameter
> to be literals, and is choking on an embedded quantified expression.  That
> said, I don't claim to understand the invariants code:  that's just what I
> could glean from the debugger and inspection of this one procedure.

That makes a lot of sense, thanks! The next question is who is at fault here --
should the invariant code be ready to process these existential conditions, or
should they have been compiled away in the normalization code.

I tend towards the latter, but unfortunately our internal contracts here are
insufficiently documented. I'll have a look at the normalization ASAP (but that
might not be very S).
msg1888 (view) Author: rpgoldman Date: 2011-11-07.21:31:54
I believe that the error in my example happens in this bit of code:


  (:action encrypt_file
   :parameters (?human - c_human
		?file - c_file
		?host - c_host
		?uid - c_uid
		?gid - c_gid
		?key - c_ekey)
   :precondition
    (and (pmode m_free)
     (at_host ?human ?host)
     (authenticated ?uid ?host)
     (in_group ?uid ?gid)
     (accessible ?file ?host)
     (and (accessible ?file ?host) 
  (in_group ?uid ?gid)
  (or (file_write ?file ?uid) 
      (file_write ?file ?gid)))
     (or (forall (?k - c_ekey)
          (not (file_encrypted ?file ?k)))
         (exists (?e - c_ekey ?n - c_info)
          (and (file_encrypted ?file ?e)
               (or (authorization ?e ?human)
                   (and (authorization ?e ?n)
                        (knows ?human ?n))))))
    )
   :effect
    (and
     (file_encrypted ?file ?key)
     (forall (?i - c_info)
      (when (file_contents_info ?file ?i)
       (encrypted ?i ?key)))
    ))

The existential is in the preconditions.  It looks like the code expects the 
existential expression to have a 'predicate' field when it does not.  Instead it 
has a set of parameters and a 'parts', which is a conjunction.

If I understand ensure_conjunction_sat, it expects all of its parts parameter to 
be literals, and is choking on an embedded quantified expression.  That said, I 
don't claim to understand the invariants code:  that's just what I could glean 
from the debugger and inspection of this one procedure.
msg1887 (view) Author: rpgoldman Date: 2011-11-07.21:00:26
FWIW, I didn't do anything to the 'nosy' list, so I think something must have gone 
awry on your end.

No apologies necessary for the bugs!  We are grateful to have fast-downward to 
use, and we hope it works for you to have us stress testing it.
msg1886 (view) Author: malte Date: 2011-11-07.19:30:02
I've added a minimal (or close to minimal) example.

This may be a case that was forgotten in the invariant synthesis and/or a
problem/unimplemented bit in the normalization code. Gabi implemented the
invariant synthesis and recently worked on something that sounds related in the
normalization code, so she'd be the best person to look into this. But she'll be
unavailable for the next two weeks. If someone else wants to have a look at this
in the meantime, that'd be good. I'll also try to find some time. Otherwise
please ping me again in two weeks and I'll discuss this with Gabi. (I'd rather
not nosy her straight away to avoid burying her under an avalanche of emails
once she's back.)

@Robert: Sorry to have you do all the alpha-testing that we should have done.
Many of the ADL-related corners of the planners are still quite murky,
unfortunately.
msg1885 (view) Author: malte Date: 2011-11-07.19:02:28
Indeed I didn't read that. :-) Splitting off the ":init" thing into a separate
issue (issue294) and renaming this one. Will have a look.
msg1883 (view) Author: erez Date: 2011-11-07.18:58:32
I think you forgot that even after getting rid of the negated atom in the initial 
state, there's still an error (which has something to do with a negated 
existential precondition, but I don't know the invariant code well enough to 
debug that).
msg1879 (view) Author: malte Date: 2011-11-07.18:49:22
Hi Erez, thanks for adding me!

@Everyone: when submitting a new issue, please always add me to the nosy list
*or* leave the nosy list empty. If the nosy list is left empty, I get an
automatic notification that there's a new issue (and the reporter is
automatically added to the nosy list). If the nosy list is set to something
nonempty, no notification is sent, so it's likely that noone will find out about
the existence of the issue unless they scan the issue list manually, which
personally I almost never do.

(Robert, if you did leave the nosy list empty, then something went wrong with
the email. There was a network interruption here recently.)

I want to eventually set up something that will send a notification about new
issues to some kind of mailing list, but it'd be a bit of work, so it may be a
while until that's done (if at all).


Regarding the issue:

Interesting, I never knew that negative literal in ":init" are allowed. But they
certainly are according to the PDDL 3.0 BNF.

They are, as Erez says, redundant, unless they clash with another positive
literal in the initial state, in which case I guess an error in the input should
be flagged. So they don't seem to serve a practical purpose. Nevertheless, we
should of course support them properly. While we're at it, we should make sure
that the following things in :init all work properly:

- literals referring incorrect atoms (undeclared predicate, wrong arity, unknown
object) should all be flagged with a useful error message
- similarly for "=" assignments referring to undeclared functions, having wrong
arity, referring to unknown objects, or unsupported value (not a non-negative
integer)
- duplicate literals or assignments should flag a warning or error; if we go for
a warning, then we should also make sure they are handled correctly
- negative literals should be allowed; if they clash with a positive literal, we
should report an error; otherwise, they can be ignored
- similarly, duplicate assignments to a fluent should flag an error if
contradictory and a warning or error if assigning the same value twice

Hope I didn't forget anything.
msg1878 (view) Author: erez Date: 2011-11-07.15:36:19
As far as I know, PDDL assumes that any fact which is not explicitly listed in 
the initial state as true - is false. There's nothing wrong with this per-se, 
it's just redundant.
I'm also nosying Malte just in case.
msg1877 (view) Author: rpgoldman Date: 2011-11-07.15:26:38
> The problem file had a negation in the initial state - (not (is_open door_1))

Should I have known that this was something to avoid using fast downward?
msg1875 (view) Author: erez Date: 2011-11-07.08:48:53
The problem file had a negation in the initial state - (not (is_open door_1))
However, even after getting rid of that, I get the following error:


Parsing... [0.050s CPU, 0.042s wall-clock]
Instantiating...
Normalizing task... [0.000s CPU, 0.006s wall-clock]
Generating Datalog program... [0.010s CPU, 0.007s wall-clock]
Normalizing Datalog program...
Normalizing Datalog program: [0.080s CPU, 0.083s wall-clock]
Preparing model... [0.030s CPU, 0.032s wall-clock]
Generated 927 rules.
Computing model... [2.530s CPU, 2.526s wall-clock]
20225 relevant atoms
72105 auxiliary atoms
92330 final queue length
167671 total queue pushes
Completing instantiation... [136.410s CPU, 136.654s wall-clock]
Instantiating: [139.090s CPU, 139.336s wall-clock]
Computing fact groups...
Finding invariants...
160 initial candidates
Traceback (most recent call last):
  File "/home/batman/hg/downward-main/src/translate/translate.py", line 568, in 
<module>
    sas_task = pddl_to_sas(task)
  File "/home/batman/hg/downward-main/src/translate/translate.py", line 447, in 
pddl_to_sas
    partial_encoding=USE_PARTIAL_ENCODING)
  File "/home/batman/hg/downward-main/src/translate/fact_groups.py", line 103, 
in compute_groups
    groups = invariant_finder.get_groups(task, reachable_action_params)
  File "/home/batman/hg/downward-main/src/translate/invariant_finder.py", line 
132, in get_groups
    invariants = list(find_invariants(task, reachable_action_params))
  File "/usr/lib/python2.6/contextlib.py", line 34, in __exit__
    self.gen.throw(type, value, traceback)
  File "/home/batman/hg/downward-main/src/translate/timers.py", line 32, in 
timing
    yield
  File "/home/batman/hg/downward-main/src/translate/invariant_finder.py", line 
132, in get_groups
    invariants = list(find_invariants(task, reachable_action_params))
  File "/home/batman/hg/downward-main/src/translate/invariant_finder.py", line 
106, in find_invariants
    if candidate.check_balance(balance_checker, enqueue_func):
  File "/home/batman/hg/downward-main/src/translate/invariants.py", line 229, 
in check_balance
    if self.operator_too_heavy(heavy_action):
  File "/home/batman/hg/downward-main/src/translate/invariants.py", line 253, 
in operator_too_heavy
    [eff2.literal.negate()])
  File "/home/batman/hg/downward-main/src/translate/invariants.py", line 64, in 
ensure_conjunction_sat
    if literal.predicate == "=": # use (in)equalities in conditions
AttributeError: 'ExistentialCondition' object has no attribute 'predicate'
msg1872 (view) Author: rpgoldman Date: 2011-11-04.20:58:05
I was just trying to use fast downward on an ADL version of one of the IPC bams-
gen domains.  
This domain/problem pair is accepted by "validate" and planned successfully by 
FF (with 
MAX_TYPES increased).

translate.py crashed with the following backtrace:

Parsing...
Traceback (most recent call last):
  File "/Users/rpg/obtw/obtw-
trunk/code/fastDownward/src/translate/translate.py", line 562, in 
<module>
    task = pddl.open()
  File 
"/System/Library/Frameworks/Python.framework/Versions/2.6/lib/python2.6/contextl
ib.py", 
line 34, in __exit__
    self.gen.throw(type, value, traceback)
  File "/Users/rpg/obtw/obtw-trunk/code/fastDownward/src/translate/timers.py", 
line 32, in 
timing
    yield
  File "/Users/rpg/obtw/obtw-
trunk/code/fastDownward/src/translate/translate.py", line 562, in 
<module>
    task = pddl.open()
  File "/Users/rpg/obtw/obtw-
trunk/code/fastDownward/src/translate/pddl/pddl_file.py", line 48, 
in open
    return tasks.Task.parse(domain_pddl, task_pddl)
  File "/Users/rpg/obtw/obtw-
trunk/code/fastDownward/src/translate/pddl/tasks.py", line 41, in 
parse
    task_name, task_domain_name, objects, init, goal, use_metric = 
parse_task(task_pddl)
  File "/Users/rpg/obtw/obtw-
trunk/code/fastDownward/src/translate/pddl/tasks.py", line 190, in 
parse_task
    initial.append(conditions.Atom(fact[0], fact[1:]))
  File "/Users/rpg/obtw/obtw-
trunk/code/fastDownward/src/translate/pddl/conditions.py", line 
274, in __init__
    self.hash = hash((self.__class__, self.predicate, self.args))
TypeError: unhashable type: 'list'

Will attach domain and problem file.
History
Date User Action Args
2011-11-14 16:03:19maltesetstatus: chatting -> resolved
nosy: + gabi
messages: + msg1937
assignedto: malte
2011-11-07 22:08:41maltesetmessages: + msg1889
2011-11-07 21:31:54rpgoldmansetmessages: + msg1888
2011-11-07 21:00:26rpgoldmansetmessages: + msg1887
2011-11-07 19:30:03maltesetfiles: + problem-issue292.pddl
messages: + msg1886
2011-11-07 19:25:24maltesetfiles: + domain-issue292.pddl
2011-11-07 19:02:28maltesetmessages: + msg1885
title: negative literal in :init should be allowed -> translator complains about something about an existential condition
2011-11-07 18:58:32erezsetmessages: + msg1883
2011-11-07 18:49:22maltesetmessages: + msg1879
title: Hash error crash in translate.py -> negative literal in :init should be allowed
2011-11-07 15:36:19erezsetnosy: + malte
messages: + msg1878
2011-11-07 15:26:38rpgoldmansetmessages: + msg1877
2011-11-07 08:48:54erezsetstatus: unread -> chatting
nosy: + erez
messages: + msg1875
2011-11-04 20:58:38rpgoldmansetfiles: + bams.pddl
2011-11-04 20:58:24rpgoldmansetfiles: + demo01.pddl
2011-11-04 20:58:06rpgoldmancreate