aiplan4eu / unified-planning

The AIPlan4EU Unified Planning Library
Apache License 2.0
181 stars 39 forks source link

Initial value not set. #569

Closed MFaisalZaki closed 3 months ago

MFaisalZaki commented 6 months ago

Describe the bug I'm trying to solve the transport domain instance 10 and the planner raise this error:

Screenshot 2024-02-21 at 10 27 26 AM

To Reproduce Here is the code to reproduce

from unified_planning.io import PDDLReader
import up_symk
from unified_planning.shortcuts import *
from unified_planning.plans import SequentialPlan

domain  = './domain.pddl'
problem = './p10.pddl'
task = PDDLReader().parse_problem(domain, problem)

plannername   = 'symk-opt'
with OneshotPlanner(name=plannername) as planner:
    result = planner.solve(task)
    seedplan = result.plan if result.status in unified_planning.engines.results.POSITIVE_OUTCOMES else SequentialPlan([], task.environment)

Additional context Here is the planning task used to trigger this error.

alvalentini commented 6 months ago

Hi @MFaisalZaki! This issue is related to #526. As explained there, this is a known limitation of the library: we do not support the PDDL "undefined".

A possible workaround is to manually set a default value for the road-length fluent:

from unified_planning.io import PDDLReader
import up_symk
from unified_planning.shortcuts import *
from unified_planning.plans import SequentialPlan

domain  = './domain.pddl'
problem = './p10.pddl'
task = PDDLReader().parse_problem(domain, problem)

expression_manager = task.environment.expression_manager
task.fluents_defaults[task.fluent("road-length")] = expression_manager.Int(-1)

plannername   = 'symk-opt'
with OneshotPlanner(name=plannername) as planner:
    result = planner.solve(task)
    seedplan = result.plan if result.status in unified_planning.engines.results.POSITIVE_OUTCOMES else SequentialPlan([], task.environment)
MFaisalZaki commented 6 months ago

Thanks @alvalentini for clarifying this.

arbimo commented 6 months ago

@alvalentini This is probably something we need to address somehow. I personally find myself often working around this by applying a patch on my up version.

The current state is fairly unsatisfactory. Adding a default is often insufficient as one might need to guard around the default value to make sure it isn't used, which may impact performance for some planners (typically lifted ones).

Also, we do not disallow undefined values in general but some methods assume there are none (e.g. initial_values). These are notably used in the PDDLPlanner implementation but not required in general. For instance, Aries would not fail in the presence of undefined values as the generation of the initial state is done on the planner side.

DillonZChen commented 6 months ago

This problem also shows up when trying to parse a problem which has static variables that are a subset of all possible ground variables. For example with this domain and problem file, we have

from unified_planning.io import PDDLReader

domain_pddl = "benchmarks/tpp/domain.pddl"
problem_pddl = "benchmarks/tpp/instances/pfile1.pddl"
reader = PDDLReader()
problem = reader.parse_problem(domain_pddl, problem_pddl)
initial_values = problem.initial_values

and stacktrace

Traceback (most recent call last):
  File "/home/dillon/code/numeric-goose/lime.py", line 7, in <module>
    initial_values = problem.initial_values
  File "/home/dillon/.conda/envs/ngoose/lib/python3.10/site-packages/unified_planning/model/mixins/initial_state.py", line 101, in initial_values
    res[f_exp] = self.initial_value(f_exp)
  File "/home/dillon/.conda/envs/ngoose/lib/python3.10/site-packages/unified_planning/model/mixins/initial_state.py", line 86, in initial_value
    raise UPProblemDefinitionError(
unified_planning.exceptions.UPProblemDefinitionError: Initial value not set for fluent: drive-cost(market1, market1)

which I assume happens because the grounder in unified planning does not notice that drive-cost(market1, market1) is unnecessary.

For my use case, I just converted the error to a warning. I'm not sure if this is a good idea for other use cases.

arbimo commented 6 months ago

which I assume happens because the grounder in unified planning does not notice that drive-cost(market1, market1) is unnecessary.

Well, as a library this is not a fact that we can leverage as we cannot assume anything about how a user might want to use the initial values.

MFaisalZaki commented 2 months ago

Hi @alvalentini, I wanted to ask if there is a way to list all missing initial values for predicates and functions.

alvalentini commented 2 months ago

Something like this should do what you are asking:

from unified_planning.model.fluent import get_all_fluent_exp

initialized_fluents = list(problem.explicit_initial_values.keys())
uninitialized_fluents = []
for f in problem.fluents:
    for fe in get_all_fluent_exp(problem, f):
        if fe not in initialized_fluents:
            uninitialized_fluents.append(fe)
MFaisalZaki commented 2 months ago

Excellent, thanks @alvalentini.

MFaisalZaki commented 2 months ago

Hi @alvalentini, L’m trying to set the initial value to false for fluents and then save the fixed problem to a new file. However this is not possible since the PDDLWriter ignores the false value.

Is there a way to set a default values for those fluents?

alvalentini commented 2 months ago

In PDDL when a predicate is not present in the (:init ) section it means that its initial value is False.