From d76209e736c9de0fabd50f1336d5ed2d6751b179 Mon Sep 17 00:00:00 2001 From: Luca Framba Date: Wed, 16 Aug 2023 10:06:40 +0200 Subject: [PATCH 1/2] Added exception when replanner remove_{action/goal} fails --- unified_planning/engines/replanner.py | 15 +++++++++++++++ unified_planning/test/test_replanner.py | 9 +++++++++ 2 files changed, 24 insertions(+) diff --git a/unified_planning/engines/replanner.py b/unified_planning/engines/replanner.py index 3db771dd3..b441b49a9 100644 --- a/unified_planning/engines/replanner.py +++ b/unified_planning/engines/replanner.py @@ -23,6 +23,7 @@ PlanGenerationResult, ) from unified_planning.engines.mixins.oneshot_planner import OptimalityGuarantee +from unified_planning.exceptions import UPUsageError from typing import Type, IO, Callable, Optional, Union, List, Tuple from fractions import Fraction @@ -108,9 +109,16 @@ def _remove_goal( (goal_exp,) = self._problem.environment.expression_manager.auto_promote(goal) goals = self._problem.goals self._problem.clear_goals() + removed = False for g in goals: if not g is goal_exp: self._problem.add_goal(g) + else: + removed = True + if not removed: + raise UPUsageError( + f"goal to remove: {goal_exp} not found inside the problem goals: {goals}" + ) def _add_action(self, action: "up.model.action.Action"): assert isinstance(self._problem, up.model.Problem) @@ -120,6 +128,13 @@ def _remove_action(self, name: str): assert isinstance(self._problem, up.model.Problem) actions = self._problem.actions self._problem.clear_actions() + removed = False for a in actions: if a.name != name: self._problem.add_action(a) + else: + removed = True + if not removed: + raise UPUsageError( + f"action to remove: {name} not found inside the problem actions: {list(map(lambda a: a.name, actions))}" + ) diff --git a/unified_planning/test/test_replanner.py b/unified_planning/test/test_replanner.py index aac31517f..4e69c66a2 100644 --- a/unified_planning/test/test_replanner.py +++ b/unified_planning/test/test_replanner.py @@ -18,6 +18,7 @@ from unified_planning.shortcuts import * from unified_planning.model.problem_kind import classical_kind from unified_planning.engines.results import POSITIVE_OUTCOMES, NEGATIVE_OUTCOMES +from unified_planning.exceptions import UPUsageError from unified_planning.test import TestCase, main from unified_planning.test import ( skipIfNoOneshotPlannerForProblemKind, @@ -67,6 +68,14 @@ def test_basic(self): res = replanner.resolve() self.assertIn(res.status, POSITIVE_OUTCOMES) + with self.assertRaises(UPUsageError): + replanner.remove_action("b") + + with self.assertRaises(UPUsageError): + y = Fluent("y") + problem.add_fluent(y, default_initial_value=False) + replanner.remove_goal(y) + @skipIfEngineNotAvailable("opt-pddl-planner") def test_robot(self): problem = self.problems["robot"].problem From d4c1db0845ff8d744e8f014fc9a81f167b7a7975 Mon Sep 17 00:00:00 2001 From: Luca Framba Date: Tue, 5 Sep 2023 09:58:42 +0200 Subject: [PATCH 2/2] feat(replanner): The replanner now uses the 2 Engine flags, skip_checks and error_on_failed_checks to determine if a useless remove actually raises an error or not. --- unified_planning/engines/mixins/replanner.py | 8 ++++++++ unified_planning/engines/replanner.py | 21 ++++++++++++-------- 2 files changed, 21 insertions(+), 8 deletions(-) diff --git a/unified_planning/engines/mixins/replanner.py b/unified_planning/engines/mixins/replanner.py index f07f49d36..b1af16a5c 100644 --- a/unified_planning/engines/mixins/replanner.py +++ b/unified_planning/engines/mixins/replanner.py @@ -111,6 +111,10 @@ def remove_goal( Removes the given goal. :param goal: the goal to remove to the problem. + :raises UPUsageError: If the goal is not found in the problem. + This works only if the checks are enabled (flag ``skip_checks``). + Based on the ``error_on_failed_checks`` this cna be an exception or a + warning. """ return self._remove_goal(goal) @@ -127,6 +131,10 @@ def remove_action(self, name: str): Removes the given action. :param action: the action to remove to the problem. + :raises UPUsageError: If the action is not found in the problem. + This works only if the checks are enabled (flag ``skip_checks``). + Based on the ``error_on_failed_checks`` this cna be an exception or a + warning. """ return self._remove_action(name) diff --git a/unified_planning/engines/replanner.py b/unified_planning/engines/replanner.py index b441b49a9..524c1626c 100644 --- a/unified_planning/engines/replanner.py +++ b/unified_planning/engines/replanner.py @@ -13,6 +13,7 @@ # limitations under the License. # +from warnings import warn import unified_planning as up import unified_planning.engines.mixins as mixins from unified_planning.model import ProblemKind @@ -115,10 +116,12 @@ def _remove_goal( self._problem.add_goal(g) else: removed = True - if not removed: - raise UPUsageError( - f"goal to remove: {goal_exp} not found inside the problem goals: {goals}" - ) + if not self._skip_checks and not removed: + msg = f"goal to remove: {goal_exp} not found inside the problem goals: {goals}" + if self._error_on_failed_checks: + raise UPUsageError(msg) + else: + warn(msg) def _add_action(self, action: "up.model.action.Action"): assert isinstance(self._problem, up.model.Problem) @@ -134,7 +137,9 @@ def _remove_action(self, name: str): self._problem.add_action(a) else: removed = True - if not removed: - raise UPUsageError( - f"action to remove: {name} not found inside the problem actions: {list(map(lambda a: a.name, actions))}" - ) + if not self._skip_checks and not removed: + msg = f"action to remove: {name} not found inside the problem actions: {list(map(lambda a: a.name, actions))}" + if self._error_on_failed_checks: + raise UPUsageError(msg) + else: + warn(msg)