From 1320e2048699780f22553b8662f5f6881f3352e7 Mon Sep 17 00:00:00 2001 From: Eugene Teoh Date: Wed, 10 Apr 2024 10:25:13 +0100 Subject: [PATCH 1/9] Fix missing joint_position_action and add gripper action --- rlbench/backend/scene.py | 36 +++++++++++++++++++--------------- tests/unit/test_environment.py | 26 ++++++++++++++++++++++++ 2 files changed, 46 insertions(+), 16 deletions(-) diff --git a/rlbench/backend/scene.py b/rlbench/backend/scene.py index 5ab3db866..b182657ed 100644 --- a/rlbench/backend/scene.py +++ b/rlbench/backend/scene.py @@ -77,7 +77,7 @@ def __init__(self, self._robot_shapes = self.robot.arm.get_objects_in_tree( object_type=ObjectType.SHAPE) - self._execute_demo_joint_position_action = None + self._joint_position_action = None def load(self, task: Task) -> None: """Loads the task and positions at the centre of the workspace. @@ -337,6 +337,8 @@ def get_demo(self, record: bool = True, demo = [] if record: self.pyrep.step() # Need this here or get_force doesn't work... + self._joint_position_action = None + gripper_open = 1.0 if self.robot.gripper.get_open_amount()[0] > 0.9 else 0.0 demo.append(self.get_observation()) while True: success = False @@ -366,7 +368,7 @@ def get_demo(self, record: bool = True, while not done: done = path.step() self.step() - self._execute_demo_joint_position_action = path.get_executed_joint_position_action() + self._joint_position_action = np.append(path.get_executed_joint_position_action(), gripper_open) self._demo_record_step(demo, record, callable_each_step) success, term = self.task.success() @@ -385,9 +387,10 @@ def get_demo(self, record: bool = True, if not contains_param: done = False while not done: - done = gripper.actuate(1.0, 0.04) - self.pyrep.step() - self.task.step() + gripper_open = 1.0 + done = gripper.actuate(gripper_open, 0.04) + self.step() + self._joint_position_action = np.append(path.get_executed_joint_position_action(), gripper_open) if self._obs_config.record_gripper_closing: self._demo_record_step( demo, record, callable_each_step) @@ -397,9 +400,10 @@ def get_demo(self, record: bool = True, if not contains_param: done = False while not done: - done = gripper.actuate(0.0, 0.04) - self.pyrep.step() - self.task.step() + gripper_open = 0.0 + done = gripper.actuate(gripper_open, 0.04) + self.step() + self._joint_position_action = np.append(path.get_executed_joint_position_action(), gripper_open) if self._obs_config.record_gripper_closing: self._demo_record_step( demo, record, callable_each_step) @@ -409,9 +413,10 @@ def get_demo(self, record: bool = True, num = float(rest[:rest.index(')')]) done = False while not done: - done = gripper.actuate(num, 0.04) - self.pyrep.step() - self.task.step() + gripper_open = num + done = gripper.actuate(gripper_open, 0.04) + self.step() + self._joint_position_action = np.append(path.get_executed_joint_position_action(), gripper_open) if self._obs_config.record_gripper_closing: self._demo_record_step( demo, record, callable_each_step) @@ -429,8 +434,8 @@ def get_demo(self, record: bool = True, # (e.g. ball rowling to goal) if not success: for _ in range(10): - self.pyrep.step() - self.task.step() + self.step() + self._joint_position_action = np.append(path.get_executed_joint_position_action(), gripper_open) self._demo_record_step(demo, record, callable_each_step) success, term = self.task.success() if success: @@ -545,8 +550,7 @@ def _get_cam_data(cam: VisionSensor, name: str): misc.update(_get_cam_data(self._cam_front, 'front_camera')) misc.update(_get_cam_data(self._cam_wrist, 'wrist_camera')) misc.update({"variation_index": self._variation_index}) - if self._execute_demo_joint_position_action is not None: + if self._joint_position_action is not None: # Store the actual requested joint positions during demo collection - misc.update({"executed_demo_joint_position_action": self._execute_demo_joint_position_action}) - self._execute_demo_joint_position_action = None + misc.update({"joint_position_action": self._joint_position_action}) return misc diff --git a/tests/unit/test_environment.py b/tests/unit/test_environment.py index eef574996..45daffec3 100644 --- a/tests/unit/test_environment.py +++ b/tests/unit/test_environment.py @@ -263,3 +263,29 @@ def test_swap_arm(self): robot_setup=robot_config) self.env.launch() self.env.shutdown() + + def test_executed_jp_action(self): + for task_cls in [ReachTarget, TakeLidOffSaucepan]: + task = self.get_task( + task_cls, JointPosition(True)) + demos = task.get_demos(5, live_demos=True) + # Check if executed joint position action is stored + for demo in demos: + jp_action = [] + self.assertTrue("joint_position_action" not in demo[0].misc) + for t, obs in enumerate(demo): + if t == 0: + # First timestep should not have an action + self.assertTrue('joint_position_action' not in obs.misc) + else: + self.assertTrue("joint_position_action" in obs.misc) + jp_action.append(obs.misc["joint_position_action"]) + + task.reset_to_demo(demo) + for t, action in enumerate(jp_action): + # action = np.append(action, 1) + obs, reward, term = task.step(action) + if term: + break + self.assertEqual(reward, 1.0) + \ No newline at end of file From 44ccb68856fede5014da461c1a3312d78c85a89e Mon Sep 17 00:00:00 2001 From: Eugene Teoh Date: Wed, 10 Apr 2024 10:30:49 +0100 Subject: [PATCH 2/9] Remove requirements.txt as it is not needed anymore --- requirements.txt | 6 ------ 1 file changed, 6 deletions(-) delete mode 100644 requirements.txt diff --git a/requirements.txt b/requirements.txt deleted file mode 100644 index 2848bc939..000000000 --- a/requirements.txt +++ /dev/null @@ -1,6 +0,0 @@ -numpy -Pillow -pyquaternion -html-testRunner -setuptools -natsort From 2b825cace2b866e9014876a96b87445cf07a920a Mon Sep 17 00:00:00 2001 From: Eugene Teoh Date: Wed, 10 Apr 2024 10:44:15 +0100 Subject: [PATCH 3/9] Change pyrep commit hash temporarily --- setup.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/setup.py b/setup.py index d6253a6b9..cdd8df40f 100644 --- a/setup.py +++ b/setup.py @@ -34,7 +34,7 @@ def get_version(rel_path): raise RuntimeError("Unable to find version string.") core_requirements = [ - "pyrep @ git+https://github.com/stepjam/PyRep.git@076ca15c57f2495a4194da03565891ab1aaa317e", + "pyrep @ git+https://github.com/eugeneteoh/PyRep.git@e42cd73c73b95d63a706b9879c0ca85a2b79b3d0", "numpy", "Pillow", "pyquaternion", From 297948a60d3df6c956a473eff442ceef1097a07a Mon Sep 17 00:00:00 2001 From: Eugene Teoh Date: Wed, 10 Apr 2024 11:44:27 +0100 Subject: [PATCH 4/9] Update pyrep hash --- setup.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/setup.py b/setup.py index cdd8df40f..bd9d4dafc 100644 --- a/setup.py +++ b/setup.py @@ -34,7 +34,7 @@ def get_version(rel_path): raise RuntimeError("Unable to find version string.") core_requirements = [ - "pyrep @ git+https://github.com/eugeneteoh/PyRep.git@e42cd73c73b95d63a706b9879c0ca85a2b79b3d0", + "pyrep @ git+https://github.com/stepjam/PyRep.git@cd9830b58ef09538562b785fc0c257f528f1762b", "numpy", "Pillow", "pyquaternion", From 29c6e7b9efd6b4afc4aa39b970692c8d601ebebf Mon Sep 17 00:00:00 2001 From: Eugene Teoh Date: Wed, 10 Apr 2024 12:56:35 +0100 Subject: [PATCH 5/9] Remove unwanted comment --- tests/unit/test_environment.py | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/unit/test_environment.py b/tests/unit/test_environment.py index 45daffec3..b432a35a8 100644 --- a/tests/unit/test_environment.py +++ b/tests/unit/test_environment.py @@ -283,7 +283,6 @@ def test_executed_jp_action(self): task.reset_to_demo(demo) for t, action in enumerate(jp_action): - # action = np.append(action, 1) obs, reward, term = task.step(action) if term: break From fcc34f5157cfb721dd7a8462bbd9ed3fe9ecc7f7 Mon Sep 17 00:00:00 2001 From: Eugene Teoh Date: Wed, 10 Apr 2024 13:10:42 +0100 Subject: [PATCH 6/9] Clean up environment for each task test --- tests/unit/test_environment.py | 44 ++++++++++++++++++---------------- 1 file changed, 23 insertions(+), 21 deletions(-) diff --git a/tests/unit/test_environment.py b/tests/unit/test_environment.py index b432a35a8..3bdd7b37b 100644 --- a/tests/unit/test_environment.py +++ b/tests/unit/test_environment.py @@ -266,25 +266,27 @@ def test_swap_arm(self): def test_executed_jp_action(self): for task_cls in [ReachTarget, TakeLidOffSaucepan]: - task = self.get_task( - task_cls, JointPosition(True)) - demos = task.get_demos(5, live_demos=True) - # Check if executed joint position action is stored - for demo in demos: - jp_action = [] - self.assertTrue("joint_position_action" not in demo[0].misc) - for t, obs in enumerate(demo): - if t == 0: - # First timestep should not have an action - self.assertTrue('joint_position_action' not in obs.misc) - else: - self.assertTrue("joint_position_action" in obs.misc) - jp_action.append(obs.misc["joint_position_action"]) - - task.reset_to_demo(demo) - for t, action in enumerate(jp_action): - obs, reward, term = task.step(action) - if term: - break - self.assertEqual(reward, 1.0) + with self.subTest(task_cls=task_cls): + task = self.get_task( + task_cls, JointPosition(True)) + demos = task.get_demos(5, live_demos=True) + # Check if executed joint position action is stored + for demo in demos: + jp_action = [] + self.assertTrue("joint_position_action" not in demo[0].misc) + for t, obs in enumerate(demo): + if t == 0: + # First timestep should not have an action + self.assertTrue('joint_position_action' not in obs.misc) + else: + self.assertTrue("joint_position_action" in obs.misc) + jp_action.append(obs.misc["joint_position_action"]) + + task.reset_to_demo(demo) + for t, action in enumerate(jp_action): + obs, reward, term = task.step(action) + if term: + break + self.assertEqual(reward, 1.0) + self.env.shutdown() \ No newline at end of file From 4f89dbe647f7f7a0ab5cadee289e4d7e812d6787 Mon Sep 17 00:00:00 2001 From: Eugene Teoh Date: Wed, 10 Apr 2024 13:16:14 +0100 Subject: [PATCH 7/9] Use pytest-xdist to parallelise tests --- .github/workflows/task_tests.yml | 3 ++- .github/workflows/unit_tests.yml | 3 ++- setup.py | 2 +- 3 files changed, 5 insertions(+), 3 deletions(-) diff --git a/.github/workflows/task_tests.yml b/.github/workflows/task_tests.yml index 56ebfee21..0c03a3864 100644 --- a/.github/workflows/task_tests.yml +++ b/.github/workflows/task_tests.yml @@ -38,4 +38,5 @@ jobs: export QT_QPA_PLATFORM_PLUGIN_PATH=$COPPELIASIM_ROOT pip install ".[dev]" - python3 -m unittest discover tests/demos + pip install "pytest-xdist[psutil]" + pytest -n auto tests/unit diff --git a/.github/workflows/unit_tests.yml b/.github/workflows/unit_tests.yml index 9d1e1e691..43164b4b6 100644 --- a/.github/workflows/unit_tests.yml +++ b/.github/workflows/unit_tests.yml @@ -39,4 +39,5 @@ jobs: export QT_QPA_PLATFORM_PLUGIN_PATH=$COPPELIASIM_ROOT pip install ".[dev]" - python3 -m unittest discover tests/unit + pip install "pytest-xdist[psutil]" + pytest -n auto tests/unit diff --git a/setup.py b/setup.py index bd9d4dafc..97425d920 100644 --- a/setup.py +++ b/setup.py @@ -60,7 +60,7 @@ def get_version(rel_path): 'rlbench.gym' ], extras_require={ - "dev": ["html-testRunner", "gym"] + "dev": ["pytest", "html-testRunner", "gym"] }, package_data={'': ['*.ttm', '*.obj', '**/**/*.ttm', '**/**/*.obj'], 'rlbench': ['task_design.ttt']}, From 93404995e6651e4e6e1676c580b35ed7ef7c94f9 Mon Sep 17 00:00:00 2001 From: Eugene Teoh Date: Wed, 10 Apr 2024 13:41:12 +0100 Subject: [PATCH 8/9] Add verbose flag for pytest --- .github/workflows/task_tests.yml | 2 +- .github/workflows/unit_tests.yml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/task_tests.yml b/.github/workflows/task_tests.yml index 0c03a3864..82b03a135 100644 --- a/.github/workflows/task_tests.yml +++ b/.github/workflows/task_tests.yml @@ -39,4 +39,4 @@ jobs: pip install ".[dev]" pip install "pytest-xdist[psutil]" - pytest -n auto tests/unit + pytest -v -n auto tests/unit diff --git a/.github/workflows/unit_tests.yml b/.github/workflows/unit_tests.yml index 43164b4b6..9142f4574 100644 --- a/.github/workflows/unit_tests.yml +++ b/.github/workflows/unit_tests.yml @@ -40,4 +40,4 @@ jobs: pip install ".[dev]" pip install "pytest-xdist[psutil]" - pytest -n auto tests/unit + pytest -v -n auto tests/unit From 1e015d209a6accbf3531233930f83e910a621e2f Mon Sep 17 00:00:00 2001 From: Eugene Teoh Date: Wed, 10 Apr 2024 16:02:58 +0100 Subject: [PATCH 9/9] Fix test being flaky due to non-determinism --- tests/unit/test_environment.py | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/tests/unit/test_environment.py b/tests/unit/test_environment.py index 3bdd7b37b..83feb08ee 100644 --- a/tests/unit/test_environment.py +++ b/tests/unit/test_environment.py @@ -269,7 +269,9 @@ def test_executed_jp_action(self): with self.subTest(task_cls=task_cls): task = self.get_task( task_cls, JointPosition(True)) - demos = task.get_demos(5, live_demos=True) + num_episodes = 20 + demos = task.get_demos(num_episodes, live_demos=True) + total_reward = 0.0 # Check if executed joint position action is stored for demo in demos: jp_action = [] @@ -287,6 +289,9 @@ def test_executed_jp_action(self): obs, reward, term = task.step(action) if term: break - self.assertEqual(reward, 1.0) + total_reward += reward + + success_rate = total_reward / num_episodes + self.assertTrue(success_rate >= 0.9) self.env.shutdown() \ No newline at end of file