Fix(FreeVarsExtractor & Oracle): Made FreeVarsExtractor and FreeVarsO…

…racle to return a FrozenSet so it can't be modified from outside

unified_planning/model/effect.py

@@ -61,7 +61,7 @@ def __init__(
         forall: Iterable["up.model.variable.Variable"] = tuple(),
     ):
         fve = fluent.environment.free_vars_extractor
-        fluents_in_fluent = fve.get(fluent)
+        fluents_in_fluent = set(fve.get(fluent))
         fluents_in_fluent.remove(fluent)
         if fluents_in_fluent:
             raise UPProblemDefinitionError(
@@ -72,15 +72,17 @@ def __init__(
         self._condition = condition
         self._kind = kind
         fvo = fluent.environment.free_vars_oracle
-        free_vars: Set["up.model.variable.Variable"] = fvo.get_free_variables(fluent)
+        free_vars: Set["up.model.variable.Variable"] = set(
+            fvo.get_free_variables(fluent)
+        )
         free_vars.update(fvo.get_free_variables(value))
         free_vars.update(fvo.get_free_variables(condition))
 
         def free_vars_without_duplicates() -> Iterator["up.model.variable.Variable"]:
             # store seen variables to avoid duplicates
             seen: Set["up.model.variable.Variable"] = set()
             for v in forall:
-                if v in free_vars and not v in seen:
+                if v in free_vars and v not in seen:
                     seen.add(v)
                     assert isinstance(
                         v, up.model.variable.Variable

unified_planning/model/variable.py

@@ -18,7 +18,7 @@
 """
 
 
-from typing import List, Optional, Set
+from typing import List, Optional, FrozenSet
 from unified_planning.environment import Environment, get_environment
 from unified_planning.model.fnode import FNode
 from unified_planning.model.operators import OperatorKind
@@ -179,36 +179,36 @@ class FreeVarsOracle(walkers.DagWalker):
     # - Other operators need to return the union of all their sons
     # - Constants have no impact
 
-    def get_free_variables(self, expression: FNode) -> Set[Variable]:
-        """Returns the set of Symbols appearing free in the expression."""
+    def get_free_variables(self, expression: FNode) -> FrozenSet[Variable]:
+        """Returns the FrozenSet of Symbols appearing free in the expression."""
         return self.walk(expression)
 
     @walkers.handles(OperatorKind.VARIABLE_EXP)
     def walk_variable_exp(
-        self, expression: FNode, args: List[Set[Variable]], **kwargs
-    ) -> Set[Variable]:
+        self, expression: FNode, args: List[FrozenSet[Variable]], **kwargs
+    ) -> FrozenSet[Variable]:
         # pylint: disable=unused-argument
-        return {expression.variable()}
+        return frozenset((expression.variable(),))
 
     @walkers.handles(OperatorKind.EXISTS, OperatorKind.FORALL)
     def walk_quantifier(
-        self, expression: FNode, args: List[Set[Variable]], **kwargs
-    ) -> Set[Variable]:
+        self, expression: FNode, args: List[FrozenSet[Variable]], **kwargs
+    ) -> FrozenSet[Variable]:
         # pylint: disable=unused-argument
         return args[0].difference(expression.variables())
 
     @walkers.handles(op.CONSTANTS)
     def walk_constant(
-        self, expression: FNode, args: List[Set[Variable]], **kwargs
-    ) -> Set[Variable]:
+        self, expression: FNode, args: List[FrozenSet[Variable]], **kwargs
+    ) -> FrozenSet[Variable]:
         # pylint: disable=unused-argument
-        return set()
+        return frozenset()
 
     @walkers.handles(
         set(OperatorKind)
         - {OperatorKind.VARIABLE_EXP, OperatorKind.EXISTS, OperatorKind.FORALL}
     )
     def walk_all(
-        self, expression: FNode, args: List[Set[Variable]], **kwargs
-    ) -> Set[Variable]:
-        return {v for s in args for v in s}
+        self, expression: FNode, args: List[FrozenSet[Variable]], **kwargs
+    ) -> FrozenSet[Variable]:
+        return frozenset(v for s in args for v in s)

unified_planning/model/walkers/free_vars.py

@@ -13,10 +13,11 @@
 # limitations under the License.
 #
 
+from itertools import chain
 import unified_planning.model.walkers as walkers
 from unified_planning.model.fnode import FNode
 from unified_planning.model.operators import OperatorKind
-from typing import List, Set
+from typing import List, FrozenSet
 
 
 class FreeVarsExtractor(walkers.dag.DagWalker):
@@ -25,21 +26,20 @@ class FreeVarsExtractor(walkers.dag.DagWalker):
     def __init__(self):
         walkers.dag.DagWalker.__init__(self)
 
-    def get(self, expression: FNode) -> Set[FNode]:
+    def get(self, expression: FNode) -> FrozenSet[FNode]:
         """
         Returns all the `fluent expressions` in the given expression.
 
         :param expression: The expression containing the `fluent expressions` to be returned.
-        :return: The set of `fluent expressions` appearing in the given expression.
+        :return: The FrozenSet of `fluent expressions` appearing in the given expression.
         """
-        ret = self.walk(expression)
-        return (
-            ret.copy()
-        )  # return a copy, otherwise modifications of the returned set will invalidate memoization
+        return self.walk(expression)
 
     @walkers.handles(OperatorKind)
-    def walk_all_types(self, expression: FNode, args: List[Set[FNode]]) -> Set[FNode]:
-        res = set(x for y in args for x in y)
+    def walk_all_types(
+        self, expression: FNode, args: List[FrozenSet[FNode]]
+    ) -> FrozenSet[FNode]:
+        res_generator = (x for y in args for x in y)
         if expression.is_fluent_exp():
-            res |= {expression}
-        return res
+            res_generator = chain(res_generator, (expression,))
+        return frozenset(res_generator)