diff --git a/.docs/PUBLISHING.md b/.docs/PUBLISHING.md index 965223d..7e14646 100644 --- a/.docs/PUBLISHING.md +++ b/.docs/PUBLISHING.md @@ -62,7 +62,7 @@ submitting**. This list is a working copy, not a substitute. |---|---| | OSI-approved licence | ✅ Apache-2.0 | | Public repository, open issue tracker, history in the open | ✅ | -| Substantial scholarly effort | ✅ Parser and grounder covering 20 of 21 PDDL requirement flags, 14 planners, LM-cut and `h^m`, PDDL 3 compiled to the classical core, learned heuristics, search tracing | +| Substantial scholarly effort | ✅ Parser and grounder covering 20 of 24 PDDL requirement flags, 14 planners, LM-cut and `h^m`, PDDL 3 compiled to the classical core, learned heuristics, search tracing | | Installation instructions | ✅ README → Install; `pip install jupyddl` | | Example usage | ✅ README, `demos/`, the browser workbench | | Automated tests | ✅ CI on Python 3.10–3.14, with and without the optional extras, plus a packaging check on the built wheel | diff --git a/AGENTS.md b/AGENTS.md index 4759cdc..93bd905 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -134,6 +134,20 @@ traps worth knowing: the planner would simply decline to notice. Those ride on conditional effects added to every domain action. +- **Durative actions are one sequential action** with three time points: + at-start conditions in the starting state, then at-start effects, then + over-all/at-end conditions (and `always`) checked in that in-between state, + then at-end effects, which win any conflict (`grounding._sequence`). What + cannot be expressed that way is refused by name, not approximated. +- **The cost model.** No metric: every action costs 1 (or its declared + `total-cost` increase). A metric over `(total-cost)`: declared increases, + 0 otherwise; increases may read *static* functions (`road-length`), which + are evaluated at grounding. A metric without `total-cost`: domain actions + are free and only preference penalties count. `(total-time)`: an action + costs its duration. `maximize` and other metric shapes are refused. +- `compile_problem` deep-copies its inputs: `ground()` must never mutate a + parsed Domain/Problem, which callers may reuse for another problem. + ### Optional task layers `Task` carries three layers that are inert unless the domain uses them, and the planners must go through the task rather than the operator to honour them: diff --git a/CHANGELOG.md b/CHANGELOG.md index 4260eb8..86ae01a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -196,6 +196,58 @@ plans because it replays them through the same model. They are now pinned by what `solve_task` actually ran, worked out by the new `jupyddl.api.resolve_heuristic`. +Wrong answers from the PDDL front end. The parser and the PDDL 3 compilations +handed the grounder a different problem from the one written, so plans +validated — against the wrong problem. Each case is now pinned by +`tests/test_frontend_semantics.py`. + +- **Durative actions merged their at-start and at-end effects**, and an add + beat a delete, so `(at start (busy)) (at end (not (busy)))` ended with + `(busy)` true. The at-end effects now apply after the at-start ones and win a + conflict, and over-all and at-end conditions are checked in the state after + the at-start effects rather than before them. `always` is checked in that + state too; the other trajectory constraints are refused when a durative + action has at-start effects, because their monitors cannot see it. +- **`sometime-after` could not be met by the final state.** An obligation + recorded by the last action's monitor could never be discharged, so the + plan `[t, r]` was reported unsolvable; the goal now accepts the response + holding at the end. +- **`forall` around a trajectory constraint moved inside it**: + `(forall (?x) (sometime (lit ?x)))` meant one moment with every `?x` lit at + once. It is now one constraint per object. A soft constraint under `forall` + crashed with an `AttributeError`; it is refused by name like the others. +- **A goal preference under `forall` kept its free variable** and could never + be satisfied; it is now one preference per object. Under `not`, `or`, `imply` + or `exists` a preference silently made the goal false; it is now an error. +- **Timed initial literals slipped past the PDDL 3 machinery.** The + `at-most-once` and `sometime-after` monitors ignored the states literals + fired in, a literal could fire after the preferences had been paid out, and + a plan could end before a literal that was already due had fired. +- **Object fluents in trajectory constraints** were read as numeric + comparisons; they are now compiled as everywhere else. Assigning one object + fluent's value to another took the function's name for an object; it is + refused. +- **The metric.** A preference the metric does not mention cost 1 to violate + rather than nothing, a metric without `(total-cost)` still charged for + actions, `maximize` was silently minimised, and `(total-time)` charged an + instantaneous action 1. `maximize`, any metric other than `(total-cost)` or + `(total-time)` plus weighted `(is-violated p)` terms, and a metric pricing a + preference the goal does not declare are now refused. +- **`ground()` edited the domain it was given**, so a second problem grounded + against the same parsed domain inherited the first one's synthetic actions + and lost the domain's own constraints. +- **Misspelt PDDL was accepted.** `:preconditon` gave an action with no + precondition and `(:goals ...)` a problem with an empty goal; unknown keys + and sections and a missing `:goal` are now errors that name them. Malformed + input — `(?x -)`, `(when (q))`, `(:goal)` — raised a bare `IndexError`; it is + a `PDDLError`, and an unbalanced parenthesis names its line. +- **Objects called `nan`, `inf` or `infinity` were read as numbers.** +- **`(increase (total-cost) (road-length ?a ?b))` was refused** although + `:action-costs` is listed as native. A cost over static functions is now + evaluated when grounding, so the IPC `transport` domains load. `:fluents` + now implies `:object-fluents`, and `:ucpop`, `:domain-axioms` and + `:safety-constraints` are refused by name rather than as unknown flags. + ## [2.4.0] ### Added diff --git a/README.md b/README.md index 0b0d7b3..a6dfe10 100644 --- a/README.md +++ b/README.md @@ -51,7 +51,7 @@ says where and what is left. - 🧩 **Hand-written PDDL parser** and grounder covering **STRIPS, full ADL, derived predicates, numeric fluents, durative actions, trajectory constraints, preferences, timed initial literals and object fluents** — 20 of - the 21 requirement flags, with the last one refused by name rather than + the 24 requirement flags, with the other four refused by name rather than ignored. ([the full matrix](#pddl-support)) - 🔎 **14 planners**: BFS, DFS, Iterative Deepening, Dijkstra, Greedy Best-First, A*, Weighted A*, IDA*, Enforced Hill Climbing, hill climbing, beam search, @@ -507,10 +507,10 @@ no heuristic at all — it prunes by *novelty* instead. ## PDDL support -**20 of the 21 requirement flags are supported.** +**20 of the 24 requirement flags are supported.** 6 are modelled natively, 7 are compiled into the core representation, and 7 are supported with a documented restriction. -Exactly 1 is **refused at parse time with an explanation** — never +The other 4 are **refused at parse time with an explanation** — never silently ignored, because a silently ignored requirement produces plans that are wrong rather than absent. @@ -521,12 +521,12 @@ jupyddl requirements --verbose # with the full compilation notes | Requirement | PDDL | Support | What jupyddl does | |---|---|---|---| -| `:action-costs` | 3.0 | **native** | `(increase (total-cost) k)` and metric minimisation. Operator costs feed straight into g, so A*/Dijkstra optimise cost. Under a metric over `(total-cost)` an action with no cost effect is free, as in IPC planners; without a metric every action costs 1. A cost inside `when` and a negative cost are refused. | +| `:action-costs` | 3.0 | **native** | `(increase (total-cost) k)` and metric minimisation. Operator costs feed straight into g, so A*/Dijkstra optimise cost. `k` is a constant or an expression over static functions, such as `(road-length ?from ?to)`, evaluated from `:init` when the action is grounded; one that reads a function some effect changes is refused. Under a metric over `(total-cost)` an action with no cost effect is free, as in IPC planners; under one over `(total-time)` an action costs its duration; under one that mentions neither, every action is free; without a metric every action costs 1 (a durative one, its duration). The metric must be `minimize` over `(total-cost)` or `(total-time)`, plus constant multiples of `(is-violated p)` and constants; `maximize` and any other shape are refused. A cost inside `when` and a negative cost are refused. | | `:conditional-effects` | 1.2 | **native** | `when` and `forall` inside effects. Kept explicitly in the grounded operator and evaluated against the state at application time. | | `:derived-predicates` | 2.2 | **native** | `(:derived ...)` axioms. Rules are grounded and re-evaluated from scratch to a least fixpoint after every state change, so planners and heuristics always see closed states and a derived fact whose support goes away is retracted. Negating a derived predicate in a precondition or goal is supported; negating one inside another rule, which needs stratification, is refused. Derived predicates may not appear in action effects. | | `:equality` | 1.2 | **native** | `=` between terms. Evaluated during grounding; infeasible instances are dropped. | | `:strips` | 1.2 | **native** | Add/delete effects over positive literals. The core representation. Everything else compiles down to this. | -| `:typing` | 1.2 | **native** | Typed parameters and objects, with type hierarchies. Subtypes are resolved transitively when building the object pools. | +| `:typing` | 1.2 | **native** | Typed parameters and objects, with type hierarchies. Subtypes are resolved transitively when building the object pools. `(either a b)` types are refused; declare a common supertype instead. | | `:adl` | 1.2 | **compiled** | The full ADL feature set. | | `:disjunctive-preconditions` | 1.2 | **compiled** | `or` in preconditions and goals. Preconditions are converted to DNF; each disjunct becomes its own grounded operator. A disjunctive goal becomes a single artificial goal fact achieved by one zero-cost operator per disjunct. | | `:duration-inequalities` | 2.1 | **compiled** | Durations bounded by inequalities rather than fixed. Bounds are collected and the shortest feasible duration is chosen. With no concurrency and no continuous change nothing in the model prefers a longer action, so the tightest lower bound is makespan-optimal. Strict `<`/`>` are refused: they have no shortest feasible value. | @@ -534,14 +534,17 @@ jupyddl requirements --verbose # with the full compilation notes | `:negative-preconditions` | 1.2 | **compiled** | `not` in preconditions and goals. Compiled to positive normal form: each negated fluent gets a complement fact that every operator maintains. | | `:quantified-preconditions` | 1.2 | **compiled** | Both `exists` and `forall` in preconditions. | | `:universal-preconditions` | 1.2 | **compiled** | `forall` in preconditions and goals. Expanded over the typed object pool into a conjunction. | -| `:constraints` | 3.0 | **partial** | State trajectory constraints. `always`, `at-end`, `sometime`, `sometime-before`, `sometime-after` and `at-most-once` are compiled into invariants on every action, monitor facts and extra goal conjuncts. The metric-time forms (`within`, `always-within`, `hold-after`, `hold-during`) are refused by name. | -| `:durative-actions` | 2.1 | **partial** | `(:durative-action ...)` with timed conditions and effects. Compiled to sequential actions: at-start and over-all conditions become the precondition, at-start and at-end effects are merged, and the duration is carried through so plans report a makespan. Actions therefore never overlap — this models sequential temporal planning, not true concurrency, so a plan needing two actions to run at the same time will not be found. | +| `:constraints` | 3.0 | **partial** | State trajectory constraints. `always`, `at-end`, `sometime`, `sometime-before`, `sometime-after` and `at-most-once` are compiled into invariants on every action, monitor facts and extra goal conjuncts; the monitors also watch the states timed literals fire in. A constraint under `forall` becomes one constraint per object. The metric-time forms (`within`, `always-within`, `hold-after`, `hold-during`) are refused by name, and so is any form but `always` and `at-end` when a durative action has at-start effects: its monitor cannot see the state between that action's start and end. | +| `:durative-actions` | 2.1 | **partial** | `(:durative-action ...)` with timed conditions and effects. Compiled to one sequential action each. At-start conditions are checked in the state the action starts in; over-all and at-end conditions in the state after its at-start effects (they are regressed through those effects when grounding). The at-start effects apply first and the at-end effects after them, so an at-end effect wins a conflict. The duration is carried through so plans report a makespan, and a `(total-time)` metric minimises it. Actions therefore never overlap — this models sequential temporal planning, not true concurrency, so a plan needing two actions to run at the same time will not be found. Refused: every effect and condition needs a time specifier (put a `when` inside one: `(at end (when c e))`); an at-start numeric effect that a later condition or effect of the same action reads; an at-end `when` that deletes what the at-start effects add, or reads a fact an at-start `when` may change. | | `:fluents` | 2.1 | **partial** | Numeric plus object fluents. Both halves are implemented; see the two rows above for what each one covers. | | `:numeric-fluents` | 2.1 | **partial** | Numeric state variables, comparisons and assignments. Supported: ground numeric fluents, comparisons (< <= = >= >) in preconditions and goals, and assign/increase/decrease/scale-up/scale-down effects over arithmetic expressions. Numeric values are part of the state, so the state space can become infinite — the delete-relaxation heuristics ignore numeric conditions, which keeps them admissible but uninformative about them. | -| `:object-fluents` | 3.1 | **partial** | Functions returning objects rather than numbers. Compiled to a predicate plus a uniqueness rule: `(= (location ?p) ?x)` becomes a fact and `assign` clears the old value first. Using an object fluent as a *nested term* — `(at ?t (location ?p))` — is refused; write the equality form instead. | -| `:preferences` | 3.0 | **partial** | Soft goals scored by a metric. Goal preferences are compiled into a priced choice: a closing action freezes the state, then each preference is resolved either for free (if it holds) or at its `(is-violated p)` weight, so cost-optimal search minimises the metric. Preferences over trajectory constraints or inside action preconditions are refused. | -| `:timed-initial-literals` | 2.2 | **partial** | Facts that become true (or false) at a given absolute time. Elapsed time becomes a numeric fluent advanced by action durations. Each literal gets a firing action guarded on the clock plus a wait action that advances it, and every domain action is blocked while a due literal has not fired. Because actions never overlap, a literal scheduled strictly inside an action's duration fires immediately after that action rather than during it. | +| `:object-fluents` | 3.1 | **partial** | Functions returning objects rather than numbers. Compiled to a predicate plus a uniqueness rule: `(= (location ?p) ?x)` becomes a fact and `assign` clears the old value first, in preconditions, effects, goals and trajectory constraints alike. Using an object fluent as a *nested term* — `(at ?t (location ?p))` — or as the value of another, as in `(assign (location ?p) (location ?q))`, is refused; write the equality form with a variable instead. | +| `:preferences` | 3.0 | **partial** | Soft goals scored by a metric. Goal preferences are compiled into a priced choice: a closing action freezes the state, then each preference is resolved either for free (if it holds) or at its `(is-violated p)` weight, so cost-optimal search minimises the metric. Timed literals cannot fire after the closing action. A preference under `forall` is one preference per object, each priced at the name's weight. Under a metric, a preference it does not mention costs nothing to violate; without one, each costs 1. Preferences over trajectory constraints, inside action preconditions, or under `or`, `not`, `imply` or `exists` are refused. | +| `:timed-initial-literals` | 2.2 | **partial** | Facts that become true (or false) at a given absolute time. Elapsed time becomes a numeric fluent advanced by action durations. Each literal gets a firing action guarded on the clock plus a wait action that advances it, and every domain action — and the end of the plan — is blocked while a due literal has not fired. Because actions never overlap, a literal scheduled strictly inside an action's duration fires immediately after that action rather than during it. A `(total-time)` metric is refused here: waiting takes time no action is charged for. | | `:continuous-effects` | 2.1 | **rejected** | Effects that change continuously over an action's duration. Requires continuous-time reasoning, which this planner does not do. | +| `:domain-axioms` | 1.2 | **rejected** | PDDL 1.2 `(:axiom ...)` blocks. Superseded by `:derived-predicates`, which is supported: write each axiom as a `(:derived ...)` rule. | +| `:safety-constraints` | 1.2 | **rejected** | PDDL 1.2 `(:safety ...)` constraints. Superseded by PDDL 3 trajectory constraints: `(:constraints (always ...))` says the same thing and is supported. | +| `:ucpop` | 1.2 | **rejected** | The PDDL 1.2 UCPOP bundle: `:adl` plus `:domain-axioms` and `:safety-constraints`. Refused because two of its parts are; declare `:adl` (and `:derived-predicates` for axioms) instead. | The table is generated from `jupyddl.requirements`, which is the single source of truth: the parser, the CLI and the web workbench all read the same registry. diff --git a/jupyddl/compile.py b/jupyddl/compile.py index 9209af1..d68c79d 100644 --- a/jupyddl/compile.py +++ b/jupyddl/compile.py @@ -14,15 +14,19 @@ from __future__ import annotations +import copy from dataclasses import replace +from itertools import product from .parser.ast import ( Action, AddEffect, And, + Arithmetic, Atom, Comparison, ConjunctiveEffect, + Constraint, DelEffect, Domain, EqualityConstraint, @@ -39,6 +43,7 @@ Predicate, Preference, Problem, + TimedEffect, Truth, UnsupportedFeatureError, WhenEffect, @@ -122,35 +127,143 @@ def _is_domain_action(action: Action) -> bool: return not action.name.startswith(PREFIX) +def _changes_the_world(action: Action) -> bool: + """Domain actions, and the firing of a timed literal. + + These are the only actions that change a fact the domain author wrote, so + every state on a plan's trajectory except the last is the state one of + them is applied in. The other synthetic actions only move the clock or + set bookkeeping facts. + """ + return _is_domain_action(action) or action.name.startswith(f"{PREFIX}fire-til-") + + def _declare(domain: Domain, name: str, params=()) -> None: if all(p.name != name for p in domain.predicates): domain.predicates.append(Predicate(name, list(params))) -def _add_precondition(domain: Domain, formula, only_domain_actions=True) -> None: - """Conjoin ``formula`` onto every action's precondition.""" +def _add_precondition( + domain: Domain, formula, only_domain_actions=True, during=False +) -> None: + """Conjoin ``formula`` onto every action's precondition. + + ``during`` also requires it of a durative action's ``late_condition``, i.e. + in the state between its at-start and at-end effects -- when there is such + a state: without at-start effects it is the state the action starts in. + """ for index, action in enumerate(domain.actions): if only_domain_actions and not _is_domain_action(action): continue - domain.actions[index] = replace( - action, precondition=_conjoin(action.precondition, formula) - ) + changes = {"precondition": _conjoin(action.precondition, formula)} + if during and _has_start_effects(action.effect): + changes["late_condition"] = _conjoin(action.late_condition, formula) + domain.actions[index] = replace(action, **changes) def _add_conditional_effect(domain: Domain, condition, body) -> None: - """Give every domain action a ``when condition body`` effect. + """Give every action that changes the world a ``when condition body`` effect. Used for monitors that must not be optional: the planner cannot decline to - notice that a constraint's trigger became true. + notice that a constraint's trigger became true. That includes a timed + literal firing: the state it fires in is on the trajectory too, and a + monitor that skipped it would miss a trigger the clock produced. """ for index, action in enumerate(domain.actions): - if not _is_domain_action(action): + if not _changes_the_world(action): continue domain.actions[index] = replace( action, effect=_also(action.effect, WhenEffect(condition, body)) ) +def _is_empty_effect(effect) -> bool: + """True when ``effect`` changes no fact and no fluent (a cost is neither).""" + if isinstance(effect, ConjunctiveEffect): + return all(_is_empty_effect(part) for part in effect.parts) + return isinstance(effect, IncreaseCostEffect) + + +def _has_start_effects(effect) -> bool: + """True for a durative action whose at-start effects are not empty.""" + if isinstance(effect, TimedEffect): + return not _is_empty_effect(effect.start) + if isinstance(effect, ConjunctiveEffect): + return any(_has_start_effects(part) for part in effect.parts) + return False + + +def _metric_mentions(problem: Problem, name: str) -> bool: + def walk(expression): + if isinstance(expression, FluentRef): + return expression.name == name + if isinstance(expression, Arithmetic): + return walk(expression.left) or walk(expression.right) + return False + + return problem.metric is not None and walk(problem.metric[1]) + + +# -------------------------------------------------------------------------- +# quantified constraints and preferences +# -------------------------------------------------------------------------- +def _object_pool(domain: Domain, problem: Problem) -> dict: + """``type -> objects``, as the grounder will build it.""" + from .grounding import _build_type_objects # grounding imports this module + + return _build_type_objects(domain, problem) + + +def _bindings(params, pool: dict): + """Every ``{var: object}`` assignment for ``params``, with its objects.""" + variables = [var for var, _ in params] + for combo in product(*(pool.get(typ, ()) for _, typ in params)): + yield dict(zip(variables, combo)), combo + + +def _substitute_term(expression, binding: dict): + if isinstance(expression, FluentRef): + if not expression.args and expression.name in binding: + return FluentRef(binding[expression.name], ()) + args = tuple(binding.get(arg, arg) for arg in expression.args) + return FluentRef(expression.name, args) + if isinstance(expression, Arithmetic): + return Arithmetic( + expression.op, + _substitute_term(expression.left, binding), + _substitute_term(expression.right, binding), + ) + return expression + + +def _substitute(formula, binding: dict): + """``formula`` with the variables in ``binding`` replaced by objects.""" + if isinstance(formula, Literal): + atom = formula.atom + args = tuple(binding.get(arg, arg) for arg in atom.args) + return Literal(Atom(atom.predicate, args), formula.positive) + if isinstance(formula, EqualityConstraint): + return EqualityConstraint( + binding.get(formula.left, formula.left), + binding.get(formula.right, formula.right), + formula.positive, + ) + if isinstance(formula, Comparison): + return Comparison( + formula.op, + _substitute_term(formula.left, binding), + _substitute_term(formula.right, binding), + ) + if isinstance(formula, (And, Or)): + return type(formula)(tuple(_substitute(p, binding) for p in formula.parts)) + if isinstance(formula, (Forall, Exists)): + # An inner quantifier shadows the outer variable of the same name. + bound = {var for var, _ in formula.params} + inner = {var: obj for var, obj in binding.items() if var not in bound} + return type(formula)(formula.params, _substitute(formula.body, inner)) + return formula + + # -------------------------------------------------------------------------- # object fluents # -------------------------------------------------------------------------- @@ -176,22 +289,37 @@ def predicate_name(name: str) -> str: params = list(fluent.params) + [("?__value", fluent.result_type)] _declare(domain, predicate_name(fluent.name), params) + def object_value(fluent: str, value) -> str: + """The object a term names, refusing anything that is not one.""" + if isinstance(value, Number): + raise UnsupportedFeatureError( + f"'{fluent}' returns an object, so it cannot be compared with " + "or set to a number" + ) + if isinstance(value, FluentRef) and (value.args or value.name in fluents): + # (loc ?q) here would be read as the *object* "loc". + raise UnsupportedFeatureError( + f"'{fluent}' is compared with or set to the value of another " + f"function, {value}; that is not supported — introduce a " + "variable for it, e.g. (and (= (loc ?q) ?x) ...)" + ) + if not isinstance(value, FluentRef): + raise UnsupportedFeatureError( + f"'{fluent}' returns an object, so arithmetic on it is meaningless" + ) + return value.name + def rewrite_condition(formula): if isinstance(formula, Comparison) and formula.op in ("=", "!="): left, right = formula.left, formula.right for a, b in ((left, right), (right, left)): if isinstance(a, FluentRef) and a.name in fluents: - if isinstance(b, Number): - raise UnsupportedFeatureError( - f"'{a.name}' returns an object, so it cannot be " - "compared with a number" - ) - value = b.name if isinstance(b, FluentRef) else str(b) if isinstance(b, FluentRef) and b.name in fluents: raise UnsupportedFeatureError( "comparing two object fluents directly is not " "supported; introduce a variable for one of them" ) + value = object_value(a.name, b) atom = _fact(predicate_name(a.name), *a.args, value) return Literal(atom, formula.op == "=") return formula @@ -208,6 +336,8 @@ def rewrite_condition(formula): def rewrite_effect(effect): if isinstance(effect, ConjunctiveEffect): return ConjunctiveEffect([rewrite_effect(p) for p in effect.parts]) + if isinstance(effect, TimedEffect): + return TimedEffect(rewrite_effect(effect.start), rewrite_effect(effect.end)) if isinstance(effect, ForallEffect): return ForallEffect(effect.params, rewrite_effect(effect.body)) if isinstance(effect, WhenEffect): @@ -222,8 +352,7 @@ def rewrite_effect(effect): ) fluent = fluents[effect.target.name] name = predicate_name(fluent.name) - value = effect.value - new = value.name if isinstance(value, FluentRef) else str(value) + new = object_value(fluent.name, effect.value) # Clear whatever the function used to return, then set the new # value: that is what keeps it a function rather than a relation. clear = ForallEffect( @@ -238,20 +367,32 @@ def rewrite_effect(effect): ) return effect + def rewrite_constraint(constraint): + if isinstance(constraint, Preference): + return replace(constraint, body=rewrite_constraint(constraint.body)) + args = tuple(rewrite_condition(arg) for arg in constraint.args) + return replace(constraint, args=args) + for index, action in enumerate(domain.actions): + late = action.late_condition domain.actions[index] = replace( action, precondition=rewrite_condition(action.precondition), effect=rewrite_effect(action.effect), + late_condition=None if late is None else rewrite_condition(late), ) for index, rule in enumerate(domain.derived): domain.derived[index] = replace(rule, body=rewrite_condition(rule.body)) problem.goal = rewrite_condition(problem.goal) for index, preference in enumerate(problem.preferences): - problem.preferences[index] = Preference( - preference.name, rewrite_condition(preference.body) + problem.preferences[index] = replace( + preference, body=rewrite_condition(preference.body) ) + # Trajectory constraints read the same terms; left alone, an equality on an + # object fluent would be taken for a numeric comparison. + domain.constraints = [rewrite_constraint(c) for c in domain.constraints] + problem.constraints = [rewrite_constraint(c) for c in problem.constraints] # `(= (location p1) depot)` in :init becomes a plain fact. for fluent_ref, value in problem.init_objects.items(): @@ -289,10 +430,18 @@ def compile_timed_initials(domain: Domain, problem: Problem): firing therefore requires every earlier literal to have fired already. Because actions do not overlap, a literal scheduled strictly inside an - action's duration fires immediately after it rather than during it. + action's duration fires immediately after it rather than during it. The + goal carries the same guards, so a plan cannot end in a state a due literal + would already have changed. """ if not problem.timed_initials: return domain, problem + if _metric_mentions(problem, "total-time"): + raise UnsupportedFeatureError( + "a (total-time) metric with timed initial literals is not supported: " + "waiting for a literal takes time that no action is charged for, so " + "the search would not minimise the makespan" + ) problem.init_numeric = dict(problem.init_numeric) problem.init_numeric.setdefault(CLOCK, 0.0) @@ -310,6 +459,9 @@ def compile_timed_initials(domain: Domain, problem: Problem): guards = [] earlier: list = [] + # "Literal j is the first one not fired yet, and it is not due": one of + # these holds exactly when every due literal has fired. + caught_up = [] for k, timed in enumerate(sorted(problem.timed_initials, key=lambda t: t.time)): marker = f"{PREFIX}til-{k}" _declare(domain, marker) @@ -343,11 +495,18 @@ def compile_timed_initials(domain: Domain, problem: Problem): ) ) guards.append(Or((not_due, _holds(marker)))) + caught_up.append(_conjoin(in_order, _absent(marker), not_due)) earlier.append(marker) + caught_up.append(_conjoin(*[_holds(name) for name in earlier])) - # A due literal blocks everything else until it has fired. + # A due literal blocks everything else until it has fired -- including the + # end of the plan. Without the goal guard, a durative action that overran a + # literal could end the plan in a state the literal would have changed. + # Literals fire in order, so the goal can say it with one disjunct per + # literal rather than the 2^k a conjunction of the guards expands to. for guard in guards: _add_precondition(domain, guard) + problem.goal = _conjoin(problem.goal, Or(tuple(caught_up))) problem.timed_initials = [] return domain, problem @@ -369,14 +528,19 @@ def compile_constraints(domain: Domain, problem: Problem): ``always (phi implies monitor)``. * ``sometime-after phi psi`` — a *forced* monitor: every action records an outstanding obligation when ``phi`` holds without ``psi``, and discharges - it when ``psi`` holds. The goal requires nothing outstanding. + it when ``psi`` holds. Monitors read the state an action is applied in, + so the final state is checked by the goal instead: ``psi`` holds there, + or nothing is outstanding and ``phi`` does not hold either. * ``at-most-once phi`` — forced monitors for "phi has held" and "phi has since stopped", plus ``always not (phi and stopped)``. Forced monitors ride on conditional effects, which the planner cannot decline; optional ones ride on actions, which it applies when convenient. The difference matters: a constraint the planner could satisfy by *not - looking* would not be a constraint. + looking* would not be a constraint. Forced monitors ride on the actions + that fire timed literals as well, for the same reason. + + A constraint under ``forall`` becomes one constraint per binding. """ constraints = list(domain.constraints) + list(problem.constraints) if not constraints: @@ -389,6 +553,21 @@ def compile_constraints(domain: Domain, problem: Problem): "preferences are supported over goals, not over trajectory " "constraints" ) + constraints = _expand_constraints(domain, problem, constraints) + + # A durative action with at-start effects passes through a state between + # its start and its end. `always` is checked there (see below), but the + # monitors only see the states actions are applied in, so they would miss + # it: refuse rather than accept a plan whose middle broke the constraint. + if any(_has_start_effects(action.effect) for action in domain.actions): + for constraint in constraints: + if constraint.kind not in ("always", "at-end"): + raise UnsupportedFeatureError( + f"a '{constraint.kind}' constraint over a domain whose " + "durative actions have at-start effects is not supported: " + "its monitor cannot see the state between an action's start " + "and end. Only 'always' and 'at-end' are" + ) goal_parts = [problem.goal] invariants = [] @@ -418,9 +597,13 @@ def compile_constraints(domain: Domain, problem: Problem): AddEffect(_fact(marker)), ) _add_conditional_effect(domain, follower, DelEffect(_fact(marker))) - goal_parts.append(_absent(marker)) - # ...and the final state must not leave a fresh obligation either. - goal_parts.append(Or((_negate(trigger), follower))) + # No monitor reads the final state, so the goal does: `psi` there + # discharges any obligation, including one the last action's + # monitor just recorded; otherwise nothing may be outstanding and + # the final state must not create a fresh one. + goal_parts.append( + Or((follower, _conjoin(_absent(marker), _negate(trigger)))) + ) elif kind == "at-most-once": phi = constraint.args[0] seen = f"{PREFIX}amo-seen-{index}" @@ -445,8 +628,9 @@ def compile_constraints(domain: Domain, problem: Problem): # actions timed initial literals compile to *change facts*. Exempting # them let a plan step through a state the invariant forbade: a literal # that clears `(safe)` at t=3 and one that restores it at t=4 fire - # back-to-back with nothing checking the state in between. - _add_precondition(domain, invariant, only_domain_actions=False) + # back-to-back with nothing checking the state in between. `during` + # also checks the state between a durative action's start and end. + _add_precondition(domain, invariant, only_domain_actions=False, during=True) goal_parts.append(invariant) problem.goal = _conjoin(*goal_parts) @@ -455,6 +639,22 @@ def compile_constraints(domain: Domain, problem: Problem): return domain, problem +def _expand_constraints(domain: Domain, problem: Problem, constraints) -> list: + """One constraint per binding of a ``forall`` around it.""" + if not any(constraint.params for constraint in constraints): + return constraints + pool = _object_pool(domain, problem) + expanded = [] + for constraint in constraints: + if not constraint.params: + expanded.append(constraint) + continue + for binding, _ in _bindings(constraint.params, pool): + args = tuple(_substitute(arg, binding) for arg in constraint.args) + expanded.append(Constraint(constraint.kind, args)) + return expanded + + def _observation_monitor(domain: Domain, index: int, formula) -> str: """A fact the planner can set, for free, whenever ``formula`` holds.""" marker = f"{PREFIX}seen-{index}" @@ -476,8 +676,9 @@ def _observation_monitor(domain: Domain, index: int, formula) -> str: def compile_preferences(domain: Domain, problem: Problem): """Turn each soft goal into a priced choice: satisfy it, or pay for it. - A closing phase makes this sound. ``__close`` ends the plan — every domain - action requires that it has *not* happened — after which each preference is + A closing phase makes this sound. ``__close`` ends the plan — every action + before it, the domain's own and the ones firing timed literals alike, + requires that it has *not* happened — after which each preference is resolved by one of two zero-parameter actions: a free one that requires the preference to hold, or one costing the metric's ``(is-violated p)`` weight that does not. Cost-optimal search then picks whichever is cheaper, which is @@ -485,13 +686,43 @@ def compile_preferences(domain: Domain, problem: Problem): Freezing the state first is the point: without it the planner could satisfy a preference halfway through and then break it, and still be paid for it. + + Under a metric, a preference it does not mention costs nothing to violate — + it has no effect on plan quality. Without a metric each costs 1. A + preference under ``forall`` is one preference per binding, each priced at + the name's weight, as ``(is-violated p)`` counts violated instances. """ + declared = {preference.name for preference in problem.preferences} + for name in problem.violation_weights: + if name not in declared: + raise PDDLError( + f"the metric prices (is-violated {name}), but the goal declares " + f"no preference named '{name}'" + ) if not problem.preferences: return domain, problem + default_weight = 0.0 if problem.metric is not None else 1.0 + instances = [] # (name, label, body) + pool = None + for preference in problem.preferences: + if not preference.params: + instances.append((preference.name, preference.name, preference.body)) + continue + if pool is None: + pool = _object_pool(domain, problem) + for binding, objects in _bindings(preference.params, pool): + label = "-".join((preference.name,) + tuple(objects)) + instances.append( + (preference.name, label, _substitute(preference.body, binding)) + ) + closed = f"{PREFIX}closed" _declare(domain, closed) - _add_precondition(domain, _absent(closed)) + # Everything stops once the plan is closed, timed literals included: a + # literal firing afterwards would change the state the preferences were + # just paid out on. + _add_precondition(domain, _absent(closed), only_domain_actions=False) domain.actions.append( Action( @@ -503,37 +734,33 @@ def compile_preferences(domain: Domain, problem: Problem): ) goal_parts = [problem.goal, _holds(closed)] - for index, preference in enumerate(problem.preferences): + for index, (name, label, body) in enumerate(instances): done = f"{PREFIX}pref-{index}" _declare(domain, done) - weight = float(problem.violation_weights.get(preference.name, 1.0)) + weight = float(problem.violation_weights.get(name, default_weight)) if weight < 0: raise PDDLError( - f"preference '{preference.name}' has a negative violation " + f"preference '{name}' has a negative violation " "weight, which would reward breaking it" ) domain.actions.append( Action( - f"{PREFIX}satisfy-{preference.name}", + f"{PREFIX}satisfy-{label}", [], - _conjoin(_holds(closed), _absent(done), preference.body), + _conjoin(_holds(closed), _absent(done), body), ConjunctiveEffect([AddEffect(_fact(done)), _free()]), ) ) + # The penalty rides on the action cost, so plain cost-optimal search + # optimises the metric without knowing what a preference is. domain.actions.append( Action( - f"{PREFIX}violate-{preference.name}", + f"{PREFIX}violate-{label}", [], _conjoin(_holds(closed), _absent(done)), - ConjunctiveEffect([AddEffect(_fact(done))]), # priced below + ConjunctiveEffect([AddEffect(_fact(done)), _increase_cost(weight)]), ) ) - # The penalty rides on the action cost, so plain cost-optimal search - # optimises the metric without knowing what a preference is. - domain.actions[-1] = replace( - domain.actions[-1], - effect=_also(domain.actions[-1].effect, _increase_cost(weight)), - ) goal_parts.append(_holds(done)) problem.goal = _conjoin(*goal_parts) @@ -554,7 +781,13 @@ def compile_problem(domain: Domain, problem: Problem): Object fluents go first because they rewrite terms everywhere else reads; preferences go last because their closing phase must sit outside the machinery the other compilations add. + + The inputs are copied first: every compilation edits the domain and problem + it is given, and a parsed domain is routinely grounded against several + problems. Compiling in place leaked one problem's synthetic actions into the + next and consumed the domain's own constraints on the first. """ + domain, problem = copy.deepcopy(domain), copy.deepcopy(problem) domain, problem = compile_object_fluents(domain, problem) domain, problem = compile_timed_initials(domain, problem) domain, problem = compile_constraints(domain, problem) diff --git a/jupyddl/grounding.py b/jupyddl/grounding.py index 8f91499..83b5cdc 100644 --- a/jupyddl/grounding.py +++ b/jupyddl/grounding.py @@ -49,6 +49,7 @@ Or, PDDLError, Problem, + TimedEffect, Truth, UnsupportedFeatureError, WhenEffect, @@ -63,6 +64,7 @@ MAX_DISJUNCTS = 20000 GOAL_FACT = Atom("__goal-reached__", ()) +TOTAL_COST = "total-cost" def _ground_atom(atom: Atom, subst: dict) -> Atom: @@ -287,6 +289,24 @@ class _RawOp: synthetic: bool = False +def _new_acc() -> dict: + """An empty accumulator for one ground action's effects.""" + return { + "add": set(), + "delete": set(), + "cond": [], + "numeric": [], + "cost": 0.0, + "has_cost": False, + # (increase (total-cost) ): evaluated once the instance is + # known, from the static functions in :init. + "cost_exprs": [], + # A durative action's at-start phase, kept for regressing its + # over-all and at-end conditions. + "start": None, + } + + def _collect_effect(eff, subst, type_objects, cpos, cneg, acc): if isinstance(eff, ConjunctiveEffect): for part in eff.parts: @@ -311,7 +331,10 @@ def _collect_effect(eff, subst, type_objects, cpos, cneg, acc): "(increase (total-cost) ...) inside a 'when' is not supported: " "an action's cost may not depend on the state" ) - acc["cost"] += eff.amount + if isinstance(eff.amount, (int, float)): + acc["cost"] += eff.amount + else: + acc["cost_exprs"].append(_ground_expression(eff.amount, subst)) acc["has_cost"] = True elif isinstance(eff, NumericEffect): if cpos or cneg: @@ -344,46 +367,259 @@ def _collect_effect(eff, subst, type_objects, cpos, cneg, acc): cneg | disjunct.neg, acc, ) + elif isinstance(eff, TimedEffect): + if cpos or cneg: # pragma: no cover - the parser never nests one + raise PDDLError("a durative action's timed effects cannot be conditional") + start, end = _new_acc(), _new_acc() + _collect_effect(eff.start, subst, type_objects, set(), set(), start) + _collect_effect(eff.end, subst, type_objects, set(), set(), end) + _sequence(start, end, acc) else: raise PDDLError(f"unexpected effect node: {eff!r}") +def _maybe_changed(phase: dict) -> set: + """Facts a phase's *conditional* effects may add or delete.""" + out: set = set() + for _, _, cadd, cdel in phase["cond"]: + out |= cadd | cdel + return out + + +def _regress(pos, neg, start: dict, what: str): + """Rewrite a condition on the state *after* ``start`` as one on the state + before it, or return None when ``start`` makes it false. + + A fact the start phase adds is true afterwards (adds win), one it deletes + is false, and anything else is read from the earlier state unchanged. A + fact a start-phase ``when`` may change depends on whether that fired, which + one conjunction cannot say: refused. + """ + maybe = _maybe_changed(start) + keep_pos, keep_neg = set(), set() + for atom in pos: + if atom in start["add"]: + continue + _refuse_if_maybe(atom, maybe, what) + if atom in start["delete"]: + return None + keep_pos.add(atom) + for atom in neg: + if atom in start["add"]: + return None + _refuse_if_maybe(atom, maybe, what) + if atom in start["delete"]: + continue + keep_neg.add(atom) + return keep_pos, keep_neg + + +def _refuse_if_maybe(atom, maybe: set, what: str) -> None: + if atom in maybe: + raise UnsupportedFeatureError( + f"{what} reads {atom}, which an at-start conditional effect may " + "change; that is not supported" + ) + + +def _refuse_numeric_overlap(written: set, read: set, what: str) -> None: + overlap = sorted(str(ref) for ref in written & read) + if overlap: + raise UnsupportedFeatureError( + f"{what} reads {', '.join(overlap)}, which an at-start effect of the " + "same action changes; that is not supported" + ) + + +def _sequence(start: dict, end: dict, acc: dict) -> None: + """Fold a durative action's at-start phase, then its at-end phase, into one. + + The at-end effects happen second, so they win a conflict: an at-start add + of something the end deletes is dropped, and an at-start delete of + something the end adds loses to the add anyway. At-end ``when`` conditions + read the state after the at-start effects, so they are regressed through + them. Two cases one operator cannot express are refused: an at-end + conditional delete of something the start adds (adds win inside one + operator, so the delete could never take effect), and numeric at-end + effects that read or write what the at-start ones wrote. + """ + end_add, end_delete = set(end["add"]), set(end["delete"]) + end_cond = [] + for cpos, cneg, cadd, cdel in end["cond"]: + regressed = _regress(cpos, cneg, start, "an at-end conditional effect") + if regressed is None: + continue # the start phase made its condition false + if not regressed[0] and not regressed[1]: + end_add |= cadd # ...or true, so it always fires + end_delete |= cdel + continue + end_cond.append((frozenset(regressed[0]), frozenset(regressed[1]), cadd, cdel)) + start_add = start["add"] - end_delete + start_cond = [ + (cpos, cneg, cadd - end_delete, cdel) + for cpos, cneg, cadd, cdel in start["cond"] + ] + may_add = set(start_add) + for _, _, cadd, _ in start_cond: + may_add |= cadd + for _, _, _, cdel in end_cond: + clash = sorted(str(atom) for atom in cdel & may_add) + if clash: + raise UnsupportedFeatureError( + f"an at-end conditional effect deletes {', '.join(clash)}, which " + "the at-start effects add; that is not supported" + ) + + written = {target for _, target, _ in start["numeric"]} + for _, target, value in end["numeric"]: + read: set = {target} + _collect_fluents(value, read) + _refuse_numeric_overlap(written, read, "a numeric at-end effect") + + acc["add"] |= start_add | end_add + acc["delete"] |= start["delete"] | end_delete + acc["cond"].extend(start_cond + end_cond) + acc["numeric"].extend(start["numeric"] + end["numeric"]) + acc["cost"] += start["cost"] + end["cost"] + acc["has_cost"] = acc["has_cost"] or start["has_cost"] or end["has_cost"] + acc["cost_exprs"].extend(start["cost_exprs"] + end["cost_exprs"]) + acc["start"] = start + + +def _late_disjuncts(action, subst, type_objects, acc) -> list: + """A durative action's over-all and at-end conditions, as pre-state DNF. + + They hold in the state after the at-start effects; regressing them through + those effects turns them into conditions on the state the action starts in, + which is all a single sequential operator can check. + """ + start = acc["start"] or _new_acc() + written = {target for _, target, _ in start["numeric"]} + what = f"an over-all or at-end condition of '{action.name}'" + out = [] + for disjunct in _dnf(action.late_condition, subst, type_objects): + regressed = _regress(disjunct.pos, disjunct.neg, start, what) + if regressed is None: + continue + read: set = set() + for comparison in disjunct.comparisons: + _collect_fluents(comparison.left, read) + _collect_fluents(comparison.right, read) + _refuse_numeric_overlap(written, read, what) + out.append(_Disjunct(regressed[0], regressed[1], list(disjunct.comparisons))) + return out + + +def _leaf_effects(effect): + """Every add, delete, numeric or cost effect inside ``effect``.""" + if isinstance(effect, ConjunctiveEffect): + for part in effect.parts: + yield from _leaf_effects(part) + elif isinstance(effect, (ForallEffect, WhenEffect)): + yield from _leaf_effects(effect.body) + elif isinstance(effect, TimedEffect): + yield from _leaf_effects(effect.start) + yield from _leaf_effects(effect.end) + else: + yield effect + + def _effect_predicates(domain: Domain) -> set: - preds: set = set() + return { + eff.atom.predicate + for action in domain.actions + for eff in _leaf_effects(action.effect) + if isinstance(eff, (AddEffect, DelEffect)) + } - def walk(eff): - if isinstance(eff, ConjunctiveEffect): - for part in eff.parts: - walk(part) - elif isinstance(eff, (AddEffect, DelEffect)): - preds.add(eff.atom.predicate) - elif isinstance(eff, (ForallEffect, WhenEffect)): - walk(eff.body) +def _assigned_functions(domain: Domain) -> set: + """Names of the numeric functions some effect changes; the rest are static.""" + return { + eff.target.name + for action in domain.actions + for eff in _leaf_effects(action.effect) + if isinstance(eff, NumericEffect) + } + + +def _cost_functions(domain: Domain) -> set: + """Names of the functions ``(increase (total-cost) )`` reads.""" + read: set = set() for action in domain.actions: - walk(action.effect) - return preds + for eff in _leaf_effects(action.effect): + if isinstance(eff, IncreaseCostEffect) and not isinstance( + eff.amount, (int, float) + ): + _collect_fluents(eff.amount, read) + return {ref.name for ref in read} -def _mentions_total_cost(expression) -> bool: +def _mentions(expression, name: str) -> bool: if isinstance(expression, FluentRef): - return expression.name == "total-cost" + return expression.name == name if isinstance(expression, Arithmetic): - return _mentions_total_cost(expression.left) or _mentions_total_cost( - expression.right - ) + return _mentions(expression.left, name) or _mentions(expression.right, name) return False +def _cost_model(problem: Problem) -> str: + """What a domain action costs, according to the metric. + + * no metric — its cost effect, else its duration, else 1: cost-optimal + means shortest, or makespan-optimal for durative actions; + * a metric over ``(total-cost)`` — its cost effect, else 0, as in IPC + planners; + * a metric over ``(total-time)`` — its duration, so the sequential + makespan is minimised (0 for an instantaneous action); + * any other metric (only preference penalties) — 0: what an action costs + does not enter the metric at all. + + The parser has already refused metrics of any other shape. + """ + if problem.metric is None: + return "default" + expression = problem.metric[1] + if _mentions(expression, "total-time"): + return "time" + if _mentions(expression, TOTAL_COST): + return "cost" + return "free" + + +def _static_value(expression, values: dict, assigned: set, action_name: str): + """Evaluate a ground cost expression from :init; None if it is undefined.""" + if isinstance(expression, Number): + return float(expression.value) + if isinstance(expression, FluentRef): + if expression.name in assigned or expression.name == TOTAL_COST: + raise UnsupportedFeatureError( + f"the cost of '{action_name}' reads {expression}, which an effect " + "changes; (increase (total-cost) ...) may only read static " + "functions, whose value is fixed by :init" + ) + return values.get(expression) + if isinstance(expression, Arithmetic): + left = _static_value(expression.left, values, assigned, action_name) + right = _static_value(expression.right, values, assigned, action_name) + if left is None or right is None: + return None + if expression.op == "+": + return left + right + if expression.op == "-": + return left - right + if expression.op == "*": + return left * right + return None if right == 0 else left / right + raise PDDLError(f"unexpected numeric expression: {expression!r}") + + def _ground_raw_operators(domain, problem, type_objects) -> list: - # Under a metric over (total-cost), an action that never increases it is - # free -- that is what the metric says, and what IPC planners do. Without - # one, every action costs 1 and cost-optimal means shortest. - action_costs = problem.metric is not None and _mentions_total_cost( - problem.metric[1] - ) + model = _cost_model(problem) + assigned = _assigned_functions(domain) raw = [] for action in domain.actions: + synthetic = action.name.startswith(SYNTHETIC_PREFIX) pools = [type_objects.get(typ, ()) for (_, typ) in action.parameters] for combo in product(*pools): subst = {var: obj for (var, _), obj in zip(action.parameters, combo)} @@ -391,27 +627,36 @@ def _ground_raw_operators(domain, problem, type_objects) -> list: if not disjuncts: continue # precondition is unsatisfiable for this instance - acc = { - "add": set(), - "delete": set(), - "cond": [], - "numeric": [], - "cost": 0.0, - "has_cost": False, - } + acc = _new_acc() _collect_effect(action.effect, subst, type_objects, set(), set(), acc) + if action.late_condition is not None: + late = _late_disjuncts(action, subst, type_objects, acc) + disjuncts = _cross(disjuncts, late) + if not disjuncts: + continue args = ",".join(combo) base = f"{action.name}({args})" if combo else action.name duration = 0.0 if action.duration is not None: duration = _constant_value(action.duration, subst, action.name) - if acc["has_cost"]: - cost = acc["cost"] - elif action_costs: + + values = [ + _static_value(expression, problem.init_numeric, assigned, action.name) + for expression in acc["cost_exprs"] + ] + if None in values: + continue # reads an undefined function: PDDL makes it inapplicable + declared = acc["cost"] + sum(values) + + # Penalties and other bookkeeping costs a compilation declared are + # always charged; what a domain action costs depends on the metric. + if acc["has_cost"] and (synthetic or model in ("default", "cost")): + cost = declared + elif model in ("cost", "free"): cost = 0 - elif duration: - # A temporal action with no explicit cost: optimise makespan. + elif model == "time" or duration: + # Makespan: a durative action costs its duration. cost = duration else: cost = 1 @@ -705,7 +950,11 @@ def reads_undefined(comparisons, values=()) -> bool: resolved_axioms.append((head, parts[0], parts[1])) # --- numeric fluents ----------------------------------------------------- - fluents: set = set(problem.init_numeric) + # A static function only action costs read, like `(road-length ?a ?b)`, was + # folded into the operator costs above; it is not a state variable. If a + # condition reads it as well, the loop below brings that reading back. + cost_only = _cost_functions(domain) + fluents: set = {ref for ref in problem.init_numeric if ref.name not in cost_only} for op, *_ in resolved: for comparison in op.comparisons: _collect_fluents(comparison.left, fluents) diff --git a/jupyddl/parser/__init__.py b/jupyddl/parser/__init__.py index 80843f6..6fb3f8b 100644 --- a/jupyddl/parser/__init__.py +++ b/jupyddl/parser/__init__.py @@ -26,6 +26,7 @@ PDDLError, Predicate, Problem, + TimedEffect, Truth, UnsupportedFeatureError, WhenEffect, @@ -67,6 +68,7 @@ "PDDLError", "Predicate", "Problem", + "TimedEffect", "Truth", "UnsupportedFeatureError", "WhenEffect", diff --git a/jupyddl/parser/ast.py b/jupyddl/parser/ast.py index 9fb8593..646571d 100644 --- a/jupyddl/parser/ast.py +++ b/jupyddl/parser/ast.py @@ -163,9 +163,13 @@ class DelEffect: @dataclass class IncreaseCostEffect: - """``(increase (total-cost) k)`` — the classical action-cost shorthand.""" + """``(increase (total-cost) k)`` — the classical action-cost shorthand. - amount: float + ``amount`` is a float, or a numeric expression over *static* functions such + as ``(road-length ?from ?to)``, which the grounder evaluates from ``:init``. + """ + + amount: object @dataclass @@ -198,6 +202,19 @@ class WhenEffect: body: object +@dataclass +class TimedEffect: + """A durative action's effects, split by when they happen. + + ``start`` applies first and ``end`` after it, so an at-end effect overrides + a conflicting at-start one. The grounder folds the two into one operator; + see :mod:`jupyddl.requirements` for exactly how. + """ + + start: object + end: object + + # --- domain / problem -------------------------------------------------------- @@ -223,7 +240,11 @@ class Action: effect: object # one of the *Effect nodes above # Set when the action came from a (:durative-action ...) block; the value is # its duration, and the compilation is documented in jupyddl.requirements. + # ``precondition`` then holds the at-start conditions, ``effect`` a + # TimedEffect, and ``late_condition`` the over-all and at-end conditions, + # which are read in the state *after* the at-start effects. duration: object = None + late_condition: object = None @dataclass @@ -284,6 +305,9 @@ class Preference: name: str body: object + # ((var, type), ...) when the preference sat under a ``forall``: it then + # stands for one preference per binding, all sharing the name. + params: tuple = () @dataclass(frozen=True) @@ -292,11 +316,15 @@ class Constraint: ``kind`` is the PDDL modal operator (``always``, ``sometime``, ``at-end``, ``at-most-once``, ``sometime-before``, ``sometime-after``) and ``args`` holds - its operand formulas, already in negation normal form. + its operand formulas, already in negation normal form. ``params`` holds the + variables of any enclosing ``forall``: the constraint stands for one copy + per binding, which is not the same as quantifying inside it — + ``(forall (?x) (sometime (p ?x)))`` lets each ``?x`` have its own moment. """ kind: str args: tuple = () + params: tuple = () @dataclass(frozen=True) diff --git a/jupyddl/parser/parser.py b/jupyddl/parser/parser.py index 0798888..397de76 100644 --- a/jupyddl/parser/parser.py +++ b/jupyddl/parser/parser.py @@ -11,6 +11,8 @@ from __future__ import annotations +import re + from ..requirements import expand as expand_requirements from ..requirements import lookup as lookup_requirement from ..requirements import unsupported_reason @@ -42,6 +44,7 @@ PDDLError, Predicate, Problem, + TimedEffect, TimedInitial, Truth, UnsupportedFeatureError, @@ -67,25 +70,26 @@ } METRIC_TIME_OPS = {"within", "always-within", "hold-after", "hold-during"} -# Timed-condition / timed-effect markers inside a durative action. -TIME_SPECIFIERS = {"at", "over"} - def _parse_typed_list(items: list) -> list: """Parse ``?x ?y - type a b`` into ``[(name, type), ...]`` (default ``object``).""" + if not isinstance(items, list): + raise PDDLError(f"expected a parenthesised list, got '{items}'") result: list = [] pending: list = [] i = 0 while i < len(items): tok = items[i] if tok == "-": + if i + 1 >= len(items): + raise PDDLError(f"'-' with no type after it in {_show(items)}") typ = items[i + 1] if isinstance(typ, list): if typ and typ[0] == "either": raise UnsupportedFeatureError( "'either' types are not supported; declare a supertype instead" ) - raise UnsupportedFeatureError("'either' types are not supported") + raise PDDLError(f"malformed type {_show(typ)} in {_show(items)}") for name in pending: result.append((name, typ)) pending = [] @@ -99,14 +103,27 @@ def _parse_typed_list(items: list) -> list: return result +# A PDDL number. ``float()`` would also take ``nan``, ``inf`` and ``infinity``, +# which are perfectly good object names. +_NUMBER = re.compile(r"[+-]?(?:\d+\.?\d*|\.\d+)(?:e[+-]?\d+)?") + + def _is_number(token) -> bool: - if isinstance(token, list): - return False - try: - float(token) - except (TypeError, ValueError): - return False - return True + return isinstance(token, str) and _NUMBER.fullmatch(token) is not None + + +def _show(expr) -> str: + """Render a token tree back as PDDL, for error messages.""" + if isinstance(expr, list): + return "(" + " ".join(_show(part) for part in expr) + ")" + return str(expr) + + +def _expect_arity(expr: list, count: int, what: str) -> None: + """Raise a PDDLError unless ``expr`` has exactly ``count`` operands.""" + if len(expr) != count + 1: + plural = "" if count == 1 else "s" + raise PDDLError(f"{what} takes {count} argument{plural}: {_show(expr)}") def _parse_expression(expr): @@ -119,15 +136,21 @@ def _parse_expression(expr): if not expr: raise PDDLError("Empty numeric expression") head = expr[0] + if isinstance(head, list): + raise PDDLError(f"malformed numeric expression: {_show(expr)}") if head in ARITHMETIC: if len(expr) == 2 and head == "-": # unary minus return Arithmetic("-", Number(0.0), _parse_expression(expr[1])) + if len(expr) < 3: + raise PDDLError(f"'{head}' needs two operands: {_show(expr)}") if len(expr) != 3: raise UnsupportedFeatureError( f"'{head}' takes exactly two arguments in a numeric expression" ) return Arithmetic(head, _parse_expression(expr[1]), _parse_expression(expr[2])) if _is_number(head): + if len(expr) != 1: + raise PDDLError(f"malformed numeric expression: {_show(expr)}") return Number(float(head)) for arg in expr[1:]: if isinstance(arg, list): @@ -138,8 +161,8 @@ def _parse_expression(expr): def _parse_atom(expr) -> Atom: - if not expr or isinstance(expr[0], list): - raise PDDLError(f"Malformed atom: {expr}") + if not isinstance(expr, list) or not expr or isinstance(expr[0], list): + raise PDDLError(f"Malformed atom: {_show(expr)}") for arg in expr[1:]: if isinstance(arg, list): raise UnsupportedFeatureError( @@ -165,6 +188,8 @@ def parse_condition(expr, positive: bool = True): return Truth(positive) head = expr[0] + if isinstance(head, list): + raise PDDLError(f"Malformed condition: {_show(expr)}") if head == "and": parts = tuple(parse_condition(sub, positive) for sub in expr[1:]) @@ -185,15 +210,18 @@ def parse_condition(expr, positive: bool = True): parse_condition(expr[2], positive), ) return Or(parts) if positive else And(parts) - if head == "forall": - params = tuple(_parse_typed_list(expr[1])) - body = parse_condition(expr[2], positive) - return Forall(params, body) if positive else Exists(params, body) - if head == "exists": + if head in ("forall", "exists"): + _expect_arity(expr, 2, f"'{head}'") params = tuple(_parse_typed_list(expr[1])) body = parse_condition(expr[2], positive) - return Exists(params, body) if positive else Forall(params, body) + universal = (head == "forall") == positive + return Forall(params, body) if universal else Exists(params, body) + if head == "preference": + raise UnsupportedFeatureError( + f"a preference is only supported in a goal, not here: {_show(expr)}" + ) if head == "=": + _expect_arity(expr, 2, "'='") # `=` is equality between terms unless either side is numeric. left, right = expr[1], expr[2] if isinstance(left, list) or isinstance(right, list) or _is_number(left): @@ -203,6 +231,7 @@ def parse_condition(expr, positive: bool = True): return comparison if positive else _negate_comparison(comparison) return EqualityConstraint(left, right, positive) if head in COMPARISONS: + _expect_arity(expr, 2, f"'{head}'") comparison = Comparison( head, _parse_expression(expr[1]), _parse_expression(expr[2]) ) @@ -217,25 +246,38 @@ def parse_effect(expr): if not expr: return ConjunctiveEffect([]) head = expr[0] + if isinstance(head, list): + raise PDDLError(f"Malformed effect: {_show(expr)}") if head == "and": return ConjunctiveEffect([parse_effect(sub) for sub in expr[1:]]) if head == "not": + _expect_arity(expr, 1, "'not'") return DelEffect(_parse_atom(expr[1])) if head == "forall": + _expect_arity(expr, 2, "'forall'") return ForallEffect(_parse_typed_list(expr[1]), parse_effect(expr[2])) if head == "when": + _expect_arity(expr, 2, "'when'") return WhenEffect(parse_condition(expr[1]), parse_effect(expr[2])) if head in ASSIGN_OPS: + _expect_arity(expr, 2, f"'{head}'") target = expr[1] + if isinstance(target, list) and (not target or isinstance(target[0], list)): + raise PDDLError(f"malformed fluent in {_show(expr)}") name = target[0] if isinstance(target, list) else target value = _parse_expression(expr[2]) - if name == TOTAL_COST and head == "increase": - # The classical action-cost shorthand: keep it as an integer cost. + if name == TOTAL_COST: + if head != "increase": + raise UnsupportedFeatureError( + f"'{head}' on (total-cost) is not supported; action costs " + "are written (increase (total-cost) k)" + ) + # The classical action-cost shorthand: a constant stays a number; + # an expression must read only static functions, and the grounder + # evaluates it per action instance (see IncreaseCostEffect). if isinstance(value, Number): return IncreaseCostEffect(value.value) - raise UnsupportedFeatureError( - "(increase (total-cost) ...) needs a constant amount" - ) + return IncreaseCostEffect(value) args = tuple(target[1:]) if isinstance(target, list) else () return NumericEffect(head, FluentRef(name, args), value) if head in ("or", "imply", "exists"): @@ -250,17 +292,88 @@ def _section_key(section) -> str: def _check_requirements(requirements) -> None: """Refuse a domain up front rather than mis-planning it later.""" for flag in requirements: + if not isinstance(flag, str): + raise PDDLError(f"malformed requirement flag {_show(flag)}") if lookup_requirement(flag) is None: raise UnsupportedFeatureError( f"unknown requirement flag '{flag}'; see jupyddl.requirements " "for the full list of recognised flags" ) - for flag in expand_requirements(requirements): + # The flags as written first, then what they imply, in a fixed order: the + # error should name the flag the author wrote, whatever the hash seed. + declared = [lookup_requirement(flag).name for flag in requirements] + implied = sorted(expand_requirements(requirements) - set(declared)) + for flag in declared + implied: reason = unsupported_reason(flag) if reason: raise UnsupportedFeatureError(reason) +# Sections a domain may contain, and ones recognised only to be refused by name. +DOMAIN_SECTIONS = { + "domain", + ":requirements", + ":types", + ":constants", + ":predicates", + ":functions", + ":action", + ":durative-action", + ":derived", + ":constraints", +} +REFUSED_DOMAIN_SECTIONS = { + ":axiom": "PDDL 1.2 (:axiom ...) blocks are not supported; write the rule " + "as (:derived ...) instead", + ":safety": "PDDL 1.2 (:safety ...) constraints are not supported; " + "(:constraints (always ...)) expresses the same thing", + ":timeless": "PDDL 1.2 (:timeless ...) facts are not supported; put them in " + "each problem's :init instead", + ":process": "PDDL+ processes are not supported", + ":event": "PDDL+ events are not supported", +} +PROBLEM_SECTIONS = { + "problem", + ":domain", + ":requirements", + ":objects", + ":init", + ":goal", + ":constraints", + ":metric", + ":length", # a PDDL 1.2 hint about plan length; it constrains nothing +} + + +def _describe(section) -> str: + """``(:action move ...)`` for an error message about that section.""" + if not isinstance(section, list) or not section: + return _show(section) + head = section[0] + if len(section) > 1 and isinstance(section[1], str): + return f"({head} {section[1]} ...)" + return f"({head} ...)" + + +def _check_section(section, allowed: set, refused: dict, kind: str) -> str: + key = _section_key(section) + if key in allowed: + return key + if key in refused: + raise UnsupportedFeatureError(refused[key]) + raise PDDLError( + f"unknown {kind} section {_describe(section)}; expected one of " + + ", ".join(sorted(allowed)) + ) + + +def _name_of(section, what: str) -> str: + # Lenient about trailing tokens: `(problem switch a-b-c)` is in the wild. + if len(section) < 2 or not isinstance(section[1], str): + raise PDDLError(f"malformed ({section[0]} ...): expected one {what} name") + return section[1] + + def parse_domain(tokens: list) -> Domain: if _section_key(tokens) != "define": raise PDDLError("Domain file must start with (define ...)") @@ -277,33 +390,46 @@ def parse_domain(tokens: list) -> Domain: object_fluents: list = [] for section in tokens[1:]: - key = _section_key(section) - if key == "domain": - name = section[1] - elif key == ":requirements": - requirements = list(section[1:]) - _check_requirements(requirements) - elif key == ":types": - for child, parent in _parse_typed_list(section[1:]): - # In a :types list, "child - parent" reads name=child, type=parent. - types[child] = parent - elif key == ":constants": - constants = _parse_typed_list(section[1:]) - elif key == ":predicates": - for pred in section[1:]: - predicates.append(Predicate(pred[0], _parse_typed_list(pred[1:]))) - elif key == ":functions": - numeric, objects = _parse_functions(section[1:]) - functions.extend(numeric) - object_fluents.extend(objects) - elif key == ":action": - actions.append(_parse_action(section)) - elif key == ":durative-action": - actions.append(_parse_durative_action(section)) - elif key == ":derived": - derived.append(_parse_derived(section)) - elif key == ":constraints": - constraints.extend(_parse_constraints(section[1])) + key = _check_section( + section, DOMAIN_SECTIONS, REFUSED_DOMAIN_SECTIONS, "domain" + ) + try: + if key == "domain": + name = _name_of(section, "domain") + elif key == ":requirements": + requirements = list(section[1:]) + _check_requirements(requirements) + elif key == ":types": + for child, parent in _parse_typed_list(section[1:]): + # In a :types list, "child - parent" reads name=child, + # type=parent. + types[child] = parent + elif key == ":constants": + constants = _parse_typed_list(section[1:]) + elif key == ":predicates": + for pred in section[1:]: + if not isinstance(pred, list) or not pred: + raise PDDLError( + f"malformed predicate declaration '{_show(pred)}'" + ) + _expect_name(pred[0], "predicate") + predicates.append(Predicate(pred[0], _parse_typed_list(pred[1:]))) + elif key == ":functions": + numeric, objects = _parse_functions(section[1:]) + functions.extend(numeric) + object_fluents.extend(objects) + elif key == ":action": + actions.append(_parse_action(section)) + elif key == ":durative-action": + actions.append(_parse_durative_action(section)) + elif key == ":derived": + derived.append(_parse_derived(section)) + elif key == ":constraints": + constraints.extend(_parse_constraints(_body_of(section))) + except IndexError as exc: # a list shorter than its keyword promised + raise PDDLError(f"malformed {_describe(section)}") from exc + if not name: + raise PDDLError("the domain has no (domain )") return Domain( name, requirements, @@ -318,6 +444,21 @@ def parse_domain(tokens: list) -> Domain: ) +def _expect_name(token, what: str) -> None: + if not isinstance(token, str): + raise PDDLError(f"expected a {what} name, got {_show(token)}") + + +def _body_of(section) -> list: + """The single body of a ``(:constraints X)`` or ``(:goal X)`` section.""" + if len(section) != 2: + raise PDDLError( + f"({section[0]} ...) takes exactly one formula; " + f"got {len(section) - 1}: {_show(section)}" + ) + return section[1] + + def _parse_functions(items): """Split a ``:functions`` block into numeric functions and object fluents. @@ -335,6 +476,8 @@ def _parse_functions(items): while index < len(items): entry = items[index] if entry == "-": + if index + 1 >= len(items) or isinstance(items[index + 1], list): + raise PDDLError("'-' in :functions must be followed by a type name") result_type = items[index + 1] index += 2 for name, params in pending: @@ -344,87 +487,129 @@ def _parse_functions(items): object_fluents.append(ObjectFluent(name, params, result_type)) pending = [] continue - if isinstance(entry, list) and entry: - pending.append((entry[0], _parse_typed_list(entry[1:]))) + if not isinstance(entry, list) or not entry: + raise PDDLError(f"malformed function declaration '{_show(entry)}'") + _expect_name(entry[0], "function") + pending.append((entry[0], _parse_typed_list(entry[1:]))) index += 1 # Anything left untyped defaults to a number, as PDDL specifies. numeric.extend(Function(name, params) for name, params in pending) return numeric, object_fluents -def _parse_action(section: list) -> Action: +def _keyword_pairs(section: list, allowed: tuple, what: str): + """Yield ``(keyword, value)`` from ``(:action name :k1 v1 :k2 v2 ...)``. + + A misspelt keyword is an error rather than a silently empty precondition: + ``:preconditon`` used to give an action that was always applicable. + """ + if len(section) < 2 or not isinstance(section[1], str): + raise PDDLError(f"({section[0]} ...) needs a name") name = section[1] + rest = section[2:] + for i in range(0, len(rest), 2): + tag = rest[i] + if tag not in allowed: + raise PDDLError( + f"unknown key {_show(tag)} in {what} '{name}'; expected " + + ", ".join(allowed) + ) + if i + 1 >= len(rest): + raise PDDLError(f"{tag} in {what} '{name}' has no value") + yield tag, rest[i + 1] + + +def _parse_action(section: list) -> Action: + name = section[1] if len(section) > 1 else "" parameters: list = [] precondition = Truth(True) effect = ConjunctiveEffect([]) - i = 2 - while i < len(section): - tag = section[i] - val = section[i + 1] + keys = (":parameters", ":precondition", ":effect") + for tag, val in _keyword_pairs(section, keys, "action"): if tag == ":parameters": parameters = _parse_typed_list(val) elif tag == ":precondition": precondition = parse_condition(val) - elif tag == ":effect": + else: effect = parse_effect(val) - i += 2 return Action(name, parameters, precondition, effect) -def _strip_time_specifier(expr): - """Unwrap ``(at start X)`` / ``(at end X)`` / ``(over all X)`` to ``X``. +def _split_timed(expr, slots: dict, action_name: str, what: str) -> None: + """File each timed part of a durative condition or effect under its marker. - The sequential compilation collapses the timeline, so the marker is dropped - once it has been validated. See :mod:`jupyddl.requirements` for what that - costs you. + ``slots`` maps ``"at start"`` / ``"at end"`` / ``"over all"`` to the list + its parts go in; a marker missing from it is not allowed here. """ - if isinstance(expr, list) and len(expr) == 3 and expr[0] in TIME_SPECIFIERS: - marker = expr[1] - if expr[0] == "at" and marker in ("start", "end"): - return expr[2] - if expr[0] == "over" and marker == "all": - return expr[2] - return expr - - -def _flatten_timed(expr, out) -> None: - if not isinstance(expr, list) or not expr: + if not isinstance(expr, list): + raise PDDLError(f"malformed {what} in durative action '{action_name}'") + if not expr: return if expr[0] == "and": for sub in expr[1:]: - _flatten_timed(sub, out) + _split_timed(sub, slots, action_name, what) + return + marker = None + if len(expr) == 3 and all(isinstance(part, str) for part in expr[:2]): + marker = f"{expr[0]} {expr[1]}" + if marker in slots: + slots[marker].append(expr[2]) return - out.append(_strip_time_specifier(expr)) + if marker in ("at start", "at end", "over all"): + raise UnsupportedFeatureError( + f"'{marker}' is not allowed on an {what} of '{action_name}'" + ) + if expr[0] in ("when", "forall") and what == "effect": + raise UnsupportedFeatureError( + f"a '{expr[0]}' around time specifiers in durative action " + f"'{action_name}' is not supported; put it inside one instead, " + f"e.g. (at end ({expr[0]} ...))" + ) + raise PDDLError( + f"the {what} {_show(expr)} of durative action '{action_name}' needs a " + "time specifier: " + ", ".join(f"({m} ...)" for m in slots) + ) def _parse_durative_action(section: list) -> Action: - """Compile ``(:durative-action ...)`` into a single sequential action.""" - name = section[1] + """Compile ``(:durative-action ...)`` into a single sequential action. + + At-start conditions become the precondition; over-all and at-end + conditions become ``late_condition``, which the grounder checks in the state + *after* the at-start effects. The effects stay split in a + :class:`TimedEffect` so the at-end ones can override the at-start ones. + """ + name = section[1] if len(section) > 1 else "" parameters: list = [] duration = None - condition_parts: list = [] - effect_parts: list = [] + early: list = [] + late: list = [] + start: list = [] + end: list = [] - i = 2 - while i < len(section): - tag = section[i] - val = section[i + 1] + keys = (":parameters", ":duration", ":condition", ":effect") + for tag, val in _keyword_pairs(section, keys, "durative action"): if tag == ":parameters": parameters = _parse_typed_list(val) elif tag == ":duration": duration = _parse_duration(val, name) elif tag == ":condition": - _flatten_timed(val, condition_parts) - elif tag == ":effect": - _flatten_timed(val, effect_parts) - i += 2 + slots = {"at start": early, "over all": late, "at end": late} + _split_timed(val, slots, name, "condition") + else: + _split_timed(val, {"at start": start, "at end": end}, name, "effect") if duration is None: raise PDDLError(f"durative action '{name}' has no :duration") - precondition = parse_condition(["and"] + condition_parts) - effect = parse_effect(["and"] + effect_parts) - return Action(name, parameters, precondition, effect, duration=duration) + return Action( + name, + parameters, + parse_condition(["and"] + early), + TimedEffect(parse_effect(["and"] + start), parse_effect(["and"] + end)), + duration=duration, + late_condition=parse_condition(["and"] + late), + ) def _parse_duration(expr, action_name: str): @@ -504,44 +689,55 @@ def visit(node): def _parse_derived(section: list) -> DerivedPredicate: """Parse ``(:derived (head ?x - t) body)``.""" + if len(section) != 3: + raise PDDLError(f"(:derived ...) takes a head and a body: {_show(section)}") head_expr = section[1] if not isinstance(head_expr, list) or not head_expr: raise PDDLError("malformed (:derived ...) head") + _expect_name(head_expr[0], "derived predicate") params = _parse_typed_list(head_expr[1:]) head = Atom(head_expr[0], tuple(var for var, _ in params)) body = parse_condition(section[2]) return DerivedPredicate(head, params, body) -def _parse_constraints(expr) -> list: - """Parse a ``(:constraints ...)`` body into :class:`Constraint` nodes.""" +def _parse_constraints(expr, params: tuple = ()) -> list: + """Parse a ``(:constraints ...)`` body into :class:`Constraint` nodes. + + ``params`` accumulates the variables of enclosing ``forall`` blocks. They + stay on the constraint rather than moving inside its operands, because the + modal operators do not distribute over a conjunction: + ``(forall (?x) (sometime (p ?x)))`` says every ``?x`` gets its own moment, + while ``(sometime (forall (?x) (p ?x)))`` needs one moment for all of them. + The compiler expands one constraint per binding once the objects are known. + """ out: list = [] - def visit(node): - if not isinstance(node, list) or not node: + def visit(node, params): + if not isinstance(node, list): + raise PDDLError(f"malformed constraint: {_show(node)}") + if not node: return head = node[0] + if not isinstance(head, str): + raise PDDLError(f"malformed constraint: {_show(node)}") if head == "and": for sub in node[1:]: - visit(sub) + visit(sub, params) return if head == "forall": - # A quantified constraint expands once the object pool exists; the - # body is what carries the modality, so recurse into it and let the - # grounder expand the quantifier inside each operand. - params = tuple(_parse_typed_list(node[1])) - for constraint in _parse_constraints(node[2]): - out.append( - Constraint( - constraint.kind, - tuple(Forall(params, arg) for arg in constraint.args), - ) - ) + _expect_arity(node, 2, "'forall'") + visit(node[2], params + tuple(_parse_typed_list(node[1]))) return if head == "preference": - name = node[1] - for constraint in _parse_constraints(node[2]): - out.append(Preference(name, constraint)) + if len(node) == 3: + name, body = node[1], node[2] + elif len(node) == 2: + name, body = "(unnamed)", node[1] + else: + raise PDDLError(f"malformed preference: {_show(node)}") + for constraint in _parse_constraints(body, params): + out.append(Preference(str(name), constraint)) return if head in METRIC_TIME_OPS: raise UnsupportedFeatureError( @@ -554,39 +750,59 @@ def visit(node): f"supported: {', '.join(sorted(CONSTRAINT_OPS))}" ) arity = 2 if head in ("sometime-before", "sometime-after") else 1 - operands = node[1 : 1 + arity] - if len(operands) != arity: - raise PDDLError(f"'{head}' takes {arity} operand(s): {node}") - out.append(Constraint(head, tuple(parse_condition(o) for o in operands))) + if len(node) != arity + 1: + raise PDDLError(f"'{head}' takes {arity} operand(s): {_show(node)}") + operands = tuple(parse_condition(o) for o in node[1:]) + out.append(Constraint(head, operands, params)) - visit(expr) + visit(expr, params) return out -def _split_preferences(expr, found: list): +def _split_preferences(expr, found: list, params: tuple = ()): """Strip ``(preference name phi)`` out of a goal, leaving the hard part. Returns the goal s-expression with every preference replaced by ``()`` - (trivially true) and appends the extracted ones to ``found``. + (trivially true) and appends ``(name, phi, params)`` to ``found``, where + ``params`` are the variables of the ``forall`` blocks around it. + + PDDL 3 only lets a preference sit under ``and`` and ``forall``. Anywhere + else it has no meaning — ``(not (preference p phi))`` used to become + ``(not ())``, a goal that is simply false. """ if not isinstance(expr, list) or not expr: return expr head = expr[0] if head == "preference": if len(expr) == 3: - found.append((expr[1], expr[2])) + found.append((str(expr[1]), expr[2], params)) elif len(expr) == 2: # an unnamed preference - found.append((f"__pref{len(found) + 1}", expr[1])) + found.append((f"__pref{len(found) + 1}", expr[1], params)) else: - raise PDDLError(f"malformed preference: {expr}") + raise PDDLError(f"malformed preference: {_show(expr)}") return [] - if head in ("and", "or", "not", "imply"): - return [head] + [_split_preferences(sub, found) for sub in expr[1:]] - if head in ("forall", "exists"): - return [head, expr[1], _split_preferences(expr[2], found)] + if head == "and": + return [head] + [_split_preferences(sub, found, params) for sub in expr[1:]] + if head == "forall": + _expect_arity(expr, 2, "'forall'") + inner = params + tuple(_parse_typed_list(expr[1])) + return [head, expr[1], _split_preferences(expr[2], found, inner)] + if _mentions_preference(expr): + raise PDDLError( + f"a preference may only appear under 'and' or 'forall' in a goal, " + f"not under '{head}': {_show(expr)}" + ) return expr +def _mentions_preference(expr) -> bool: + if not isinstance(expr, list): + return False + if expr and expr[0] == "preference": + return True + return any(_mentions_preference(sub) for sub in expr) + + def _parse_timed_initial(fact): """Parse ``(at