diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md index 726a2230..d8958c9c 100644 --- a/.github/copilot-instructions.md +++ b/.github/copilot-instructions.md @@ -1 +1,6 @@ -When doing a code review, keep your suggestions focused on real issues in readability, maintainability, performance, security, and adherence to best practices. Avoid suggesting trivial minor issues like unused imports, American vs British spelling or minor formatting tweaks unless they significantly impact the quality of the code. \ No newline at end of file +# Copilot Instructions + +- Do not comment on docstring grammar or punctuation. +- Focus on logic, correctness, and API design +- Assume Black and Ruff enforce formatting +- Ignore trivial, non functional changes, namely unused imports and formatting changes. \ No newline at end of file diff --git a/utama_core/run/strategy_runner.py b/utama_core/run/strategy_runner.py index 790a442d..28c7ab6b 100644 --- a/utama_core/run/strategy_runner.py +++ b/utama_core/run/strategy_runner.py @@ -500,7 +500,7 @@ def run_test( signal.signal(signal.SIGINT, self._handle_sigint) passed = True - n_episodes = test_manager.get_n_episodes() + n_episodes = test_manager.n_episodes if not rsim_headless and self.rsim_env: self.rsim_env.render_mode = "human" if self.sim_controller is None: diff --git a/utama_core/tests/common/abstract_test_manager.py b/utama_core/tests/common/abstract_test_manager.py index 25ffb868..10025d59 100644 --- a/utama_core/tests/common/abstract_test_manager.py +++ b/utama_core/tests/common/abstract_test_manager.py @@ -17,9 +17,19 @@ class TestingStatus(Enum): class AbstractTestManager(ABC): + """Abstract base class for test managers to run strategy tests.""" + + ### To be specified in your test manager ### + n_episodes: int # number of episodes for the test + ############################################ + + def __init_subclass__(cls): + super().__init_subclass__() + if not hasattr(cls, "n_episodes"): + raise NotImplementedError(f"Subclass {cls.__name__} must define 'n_episodes' class attribute.") + def __init__(self): - # change episode_i to current_episode_i - self.episode_i = 0 + self.current_episode_number = 0 self.my_strategy: AbstractStrategy = None self.opp_strategy: AbstractStrategy = None @@ -28,22 +38,33 @@ def load_strategies(self, my_strategy: AbstractStrategy, opp_strategy: AbstractS self.my_strategy = my_strategy self.opp_strategy = opp_strategy - def update_episode_n(self, episode_i: int): + def update_episode_n(self, current_episode_number: int): """Method is used to sync test_manager on the iteration number that strategyRunner thinks it is on.""" - self.episode_i = episode_i + self.current_episode_number = current_episode_number + + ### START OF FUNCTIONS TO BE IMPLEMENTED FOR YOUR MANAGER ### @abstractmethod def reset_field(self, sim_controller: AbstractSimController, game: Game): - """Method is called at start of each test episode in strategyRunner.run_test Reset position of robots and ball - for the next strategy test.""" + """ + Method is called at start of each test episode in strategyRunner.run_test(). + Use this to reset position of robots and ball for the next episode. + Args: + sim_controller (AbstractSimController): The simulation controller to manipulate robot positions. + game (Game): The current game state. + """ ... @abstractmethod def eval_status(self, game: Game) -> TestingStatus: - """Method is called on each iteration in strategyRunner.run_test Evaluate the status of the test episode.""" - ... + """ + Method is called on each iteration in strategyRunner.run_test Evaluate the status of the test episode. - @abstractmethod - def get_n_episodes(self) -> int: - """Method is called at start of strategyRunner.run_test Get the number of episodes to run for the test.""" + Returns the current status of the test episode: + - TestingStatus.SUCCESS: test passed (terminate the episode with success) + - TestingStatus.FAILURE: test failed (terminate the episode with failure) + - TestingStatus.IN_PROGRESS: test still ongoing (continue running the episode) + """ ... + + ### END OF FUNCTIONS TO BE IMPLEMENTED FOR YOUR MANAGER ### diff --git a/utama_core/tests/motion_planning/multiple_robots_test.py b/utama_core/tests/motion_planning/multiple_robots_test.py index cd71793c..27287a4b 100644 --- a/utama_core/tests/motion_planning/multiple_robots_test.py +++ b/utama_core/tests/motion_planning/multiple_robots_test.py @@ -32,10 +32,11 @@ class MultiRobotScenario: class MultiRobotTestManager(AbstractTestManager): """Test manager for multiple robots with collision detection.""" + n_episodes = 1 + def __init__(self, scenario: MultiRobotScenario): super().__init__() self.scenario = scenario - self.n_episodes = 1 self.all_reached = False self.collision_detected = False self.min_distance = float("inf") @@ -151,9 +152,6 @@ def eval_status(self, game: Game): return TestingStatus.IN_PROGRESS - def get_n_episodes(self): - return self.n_episodes - def test_mirror_swap( headless: bool, diff --git a/utama_core/tests/motion_planning/random_movement_test.py b/utama_core/tests/motion_planning/random_movement_test.py index d4c6f945..c3afb3c7 100644 --- a/utama_core/tests/motion_planning/random_movement_test.py +++ b/utama_core/tests/motion_planning/random_movement_test.py @@ -38,10 +38,11 @@ class RandomMovementScenario: class RandomMovementTestManager(AbstractTestManager): """Test manager for random movement with collision detection.""" + n_episodes = 1 + def __init__(self, scenario: RandomMovementScenario): super().__init__() self.scenario = scenario - self.n_episodes = 1 self.collision_detected = False self.min_distance = float("inf") self.targets_reached_count: Dict[int, int] = {} @@ -122,9 +123,6 @@ def eval_status(self, game: Game): return TestingStatus.IN_PROGRESS - def get_n_episodes(self): - return self.n_episodes - def update_target_reached(self, robot_id: int): """Called by strategy when a robot reaches a target.""" if robot_id in self.targets_reached_count: diff --git a/utama_core/tests/motion_planning/single_robot_moving_obstacle_test.py b/utama_core/tests/motion_planning/single_robot_moving_obstacle_test.py index 842907ba..06695b4b 100644 --- a/utama_core/tests/motion_planning/single_robot_moving_obstacle_test.py +++ b/utama_core/tests/motion_planning/single_robot_moving_obstacle_test.py @@ -44,11 +44,12 @@ class MovingObstacleScenario: class MovingObstacleTestManager(AbstractTestManager): """Test manager that validates dynamic obstacle avoidance and target completion.""" + n_episodes = 1 + def __init__(self, scenario: MovingObstacleScenario, robot_id: int): super().__init__() self.scenario = scenario self.robot_id = robot_id - self.n_episodes = 1 self.endpoint_reached = False self.collision_detected = False self.min_obstacle_distance = float("inf") @@ -137,9 +138,6 @@ def eval_status(self, game: Game): return TestingStatus.IN_PROGRESS - def get_n_episodes(self): - return self.n_episodes - @pytest.mark.parametrize( "obstacle_scenario", diff --git a/utama_core/tests/motion_planning/single_robot_static_obstacle_test.py b/utama_core/tests/motion_planning/single_robot_static_obstacle_test.py index 0ad36c15..9ee3425a 100644 --- a/utama_core/tests/motion_planning/single_robot_static_obstacle_test.py +++ b/utama_core/tests/motion_planning/single_robot_static_obstacle_test.py @@ -30,11 +30,12 @@ class CollisionAvoidanceScenario: class CollisionAvoidanceTestManager(AbstractTestManager): """Test manager that validates obstacle avoidance and target completion.""" + n_episodes = 1 + def __init__(self, scenario: CollisionAvoidanceScenario, robot_id: int): super().__init__() self.scenario = scenario self.robot_id = robot_id - self.n_episodes = 1 self.endpoint_reached = False self.collision_detected = False self.min_obstacle_distance = float("inf") @@ -120,9 +121,6 @@ def eval_status(self, game: Game): return TestingStatus.IN_PROGRESS - def get_n_episodes(self): - return self.n_episodes - @pytest.mark.parametrize( "obstacle_config",