Effects and preconditions are sharing `free_vars` sets

[chmeldax]

There seems to be an issue with determining free variables (unbounded variables) when parsing certain domains that contain quantifiers and the same predicates are specified in the effects and preconditions.

Steps to reproduce:

An example domain would be:

    (define (domain test_domain)
        (:requirements :strips :typing :negative-preconditions)

        (:types 
            object-1 object-2 - object
            object-2-1 - object-2
            object-2-1-1 object-2-1-2 - object-2-1
        )

        (:predicates
            (predicate-1 ?object-2-1-2 - object-2-1-2 ?object-2-1-1 - object-2-1-1)
            (predicate-2)
            (predicate-3 ?object-1 - object-1 ?object-2-1-1 - object-2-1-1)
        )

        (:action action-1
            :parameters (?object-1 - object-1 ?object-2-1 - object-2-1-2)
            :effect (and
                (forall (?object-2-1-1 - object-2-1-1)
                    (when (predicate-1 ?object-2-1 ?object-2-1-1)(and (not (predicate-2))(predicate-3 ?object-1 ?object-2-1-1)))
                )
            )
        )

        (:action action-2
             :parameters ()
             :precondition (and (not (predicate-2))
                                (forall (?object-2-1 - object-2-1-1 ?object-1 - object-1)(not (predicate-3 ?object-1 ?object-2-1)))
                           )
             :effect (predicate-2)
        )
)

A call to PDDLReader().parse_problem_string(pddl_string) then fails with this message:

unified_planning.exceptions.UPUnboundedVariablesError: The precondition ((not predicate-2) and Forall (object-2-1-1 - object-2-1 object-2-1, object-1 object-1) (not predicate-3(object-1, object-2-1))) has unbounded variables:  {object-2-1-1 - object-2-1 object-2-1-1}

Expected behaviour:

A valid Problem object should be generated.

Investigation:

I have dug into this and it seems there is an issue with the way the library works with FreeVarsOracle:

1. add_effect method uses FreeVarsOracle to find free variables.
2. FreeVarsOracle uses memoization dictionary to store the results (in the form of sets).
3. when add_effect retrieves the stored results, it gets a reference to a set stored in the memoization dictionary and modifies it directly. In our case predicate-2 expression is assigned a set {object-2-1-1 - object-2-1 object-2-1-1}.
4. add_precondition uses the very same instance of FreeVarsOracle. Therefore, when retrieving free variables, it retrieves it from the said memoization dictionary. Therefore, the value already modified by add_effect is retrieved.
5. problem generation fails.

How to fix it:

It seems it could be fixed by explicitly copying free_vars in the add_effect, i.e. https://github.com/aiplan4eu/unified-planning/blob/master/unified_planning/model/action.py#L308 could be modified as follows:

free_vars: Set["up.model.variable.Variable"] = fvo.get_free_variables(fluent).copy()

It certainly fixed the problem for us. I am not sure if this might break something else, though.

[Framba-Luca]

Hi @chmeldax , thank you for spotting this and for the analysis!

I fixed it in the PR linked above by returning FrozenSets instead of Sets in the walkers.

Let me know if the PR #595 fixes it for you! Thanks again

[chmeldax]

Hi @Framba-Luca, thank you for fixing this in such short span of time!

I have just tested your PR with our domain/problem and I can confirm that it does fix the issue.

Thank you again.

[alvalentini]

I close the issue since it is fixed in #595