HorizonRobotics / SocialRobot

Apache License 2.0
71 stars 20 forks source link

Add simple internal states for goal task #117

Closed le-horizon closed 4 years ago

le-horizon commented 4 years ago

Dear reviewers,

This PR includes several changes, submitting for review before it gets too large.

1) added a mode of simple full world states as input to the agent. The input is basically the goal and the distraction object's location in the agent's own viewpoint/coordinate-system. Also includes a mode to limit the view angle of the agent. This imitates perfect attention based on visual input, and is helpful in debugging system performance.

2) a few optimizations and fixes for the task: a) place goal task objects within the boundary of the walls, b) added distraction objects' states to observation by default, c) allow using language input (without language, goal location is always the first in the input, followed by locations of the distractions, even when goal object changes e.g. from ball to car_wheel; with language, input locations are always ordered by the fixed order of objects in the world, and language specifies which of them is the goal), d) using velocity (PID) control for pioneer by default, which cuts training time to half.

Thanks, Le

Jialn commented 4 years ago

LGTM now! : )

le-horizon commented 4 years ago

@emailweixu looks like I still need your approval to merge.

Could you take a look?

Thanks, Le

Jialn commented 4 years ago

the last commit fails

ERROR
128
test_pr2 (pr2_test.TestPr2) ... [Wrn] [IntrospectionManager.cc:115] Item [data://world/default/model/ground_plane?p=pose3d/world_pose] already registered
129
[Wrn] [IntrospectionManager.cc:115] Item [data://world/default/model/ground_plane?p=vector3d/world_linear_velocity] already registered
130
[Wrn] [IntrospectionManager.cc:115] Item [data://world/default/model/ground_plane?p=vector3d/world_angular_velocity] already registered
131
[Wrn] [IntrospectionManager.cc:115] Item [data://world/default/model/ground_plane?p=vector3d/world_linear_acceleration] already registered
132
[Wrn] [IntrospectionManager.cc:115] Item [data://world/default/model/ground_plane?p=vector3d/world_angular_acceleration] already registered
133
[Wrn] [IntrospectionManager.cc:115] Item [data://world/default/model/ground_plane/link/link?p=pose3d/world_pose] already registered
134
[Wrn] [IntrospectionManager.cc:115] Item [data://world/default/model/ground_plane/link/link?p=vector3d/world_linear_velocity] already registered
135
[Wrn] [IntrospectionManager.cc:115] Item [data://world/default/model/ground_plane/link/link?p=vector3d/world_angular_velocity] already registered
136
[Wrn] [IntrospectionManager.cc:115] Item [data://world/default/model/ground_plane/link/link?p=vector3d/world_linear_acceleration] already registered
137
[Wrn] [IntrospectionManager.cc:115] Item [data://world/default/model/ground_plane/link/link?p=vector3d/world_angular_acceleration] already registered
138
python3: /usr/include/boost/smart_ptr/shared_ptr.hpp:734: typename boost::detail::sp_member_access<T>::type boost::shared_ptr<T>::operator->() const [with T = gazebo::physics::Entity; typename boost::detail::sp_member_access<T>::type = gazebo::physics::Entity*]: Assertion `px != 0' failed.
139
Aborted (core dumped)
140
##[error]Process completed with exit code 134.
le-horizon commented 4 years ago

Yes, looks like I can't run the test remotely. I'll get to it coming Monday. It's not so clear to me why it's failing.. The change shouldn't cause insertion of duplicate objects..

Jialn commented 4 years ago

I've run it locally, seems the problem is that some locations (agent_loc/model_loc) is not converted to ndarray.

  File "/home/ubuntu/Srcs/SocialRobot/SocialRobot/python/social_bot/envs/play_ground.py", line 186, in __init__
    self.reset()
  File "/home/ubuntu/Srcs/SocialRobot/SocialRobot/python/social_bot/envs/play_ground.py", line 203, in reset
    teacher_action = self._teacher.teach("")
  File "/home/ubuntu/Srcs/SocialRobot/SocialRobot/python/social_bot/teacher.py", line 335, in teach
    teacher_action = g.teach(agent_sentence)
  File "/home/ubuntu/Srcs/SocialRobot/SocialRobot/python/social_bot/teacher.py", line 88, in teach
    teacher_action = task.send(agent_sentence)
  File "/home/ubuntu/Srcs/SocialRobot/SocialRobot/python/social_bot/tasks.py", line 354, in run
    self._move_goal(goal, loc, agent_dir)
  File "/home/ubuntu/Srcs/SocialRobot/SocialRobot/python/social_bot/tasks.py", line 464, in _move_goal
    avoid_locations=avoid_locations)
  File "/home/ubuntu/Srcs/SocialRobot/SocialRobot/python/social_bot/tasks.py", line 501, in _move_obj
    dist = np.linalg.norm(loc - avoid_loc)
TypeError: unsupported operand type(s) for -: 'tuple' and 'tuple'
  In call to configurable 'PlayGround' (<function PlayGround.__init__ at 0x7fab9b133730>)
le-horizon commented 4 years ago

You are right Jiangtao! I fixed the return value to be np array, and now tests are passing.

Jialn commented 4 years ago

Hi Le, there are many warnings in "checking code style" in action log like below, both the newly added codes and existing codes. Perhaps we can run "pre-commit run -a" to auto-format them?

@@ -459,13 +466,19 @@ class GoalTask(Task):
             for item, _ in distractions.items():
                 distraction = self._world.get_agent(item)
                 loc = self._move_obj(
-                    obj=distraction, agent_loc=agent_loc,
-                    agent_dir=agent_dir, is_goal=False,
+                    obj=distraction,
+                    agent_loc=agent_loc,
+                    agent_dir=agent_dir,
+                    is_goal=False,
                     avoid_locations=avoid_locations)
                 avoid_locations.append(loc)

-    def _move_obj(
-        self, obj, agent_loc, agent_dir, is_goal=True, avoid_locations=[]):
+    def _move_obj(self,
+                  obj,
+                  agent_loc,
+                  agent_dir,
+                  is_goal=True,
+                  avoid_locations=[]):
le-horizon commented 4 years ago

Good point Jiangtao! I have precommit hooks setup for alf, but not for socialrobot. Is "pre-commit install" the way to go?

Btw, pushed the style fix.

emailweixu commented 4 years ago

Besides the change of GoalTask, some other changes of alf also cause the problem. I fixed them in the updated PR.

On Fri, Feb 14, 2020 at 9:43 AM le-horizon notifications@github.com wrote:

@le-horizon commented on this pull request.

In python/social_bot/tasks.py https://github.com/HorizonRobotics/SocialRobot/pull/117#discussion_r379560629 :

  • if not additional_observation_list:
  • additional_observation_list = self._object_list

@emailweixu https://github.com/emailweixu I tested without these two lines, still getting the same error as with the lines:

File "/home/lezhao/alf/alf/algorithms/misc_algorithm.py", line 163, in train_step add_batch() File "/home/lezhao/alf/alf/algorithms/misc_algorithm.py", line 160, in add_batch self._buffer.add_batch(feature_reshaped_tran) File "/home/lezhao/alf/alf/utils/data_buffer.py", line 94, in add_batch indices, tf.stop_gradient(bat[-n:])), self._buffer, batch) File "/home/lezhao/venv/lib/python3.5/site-packages/tensorflow_core/python/util/nest.py", line 535, in map_structure structure[0], [func(x) for x in entries], File "/home/lezhao/venv/lib/python3.5/site-packages/tensorflow_core/python/util/nest.py", line 535, in structure[0], [func(x) for x in entries], File "/home/lezhao/alf/alf/utils/data_buffer.py", line 94, in indices, tf.stop_gradient(bat[-n:])), self._buffer, batch) File "/home/lezhao/venv/lib/python3.5/site-packages/tensorflow_core/python/ops/resource_variable_ops.py", line 1180, in scatter_nd_update name=name)) File "/home/lezhao/venv/lib/python3.5/site-packages/tensorflow_core/python/ops/gen_state_ops.py", line 680, in resource_scatter_nd_update _six.raise_from(_core._status_to_exception(e.code, message), None) File "", line 3, in raise_from tensorflow.python.framework.errors_impl.InvalidArgumentError: Must have updates.shape = indices.shape[:batch_dim] + params_shape[slice_dim:], got updates.shape: [1,4,23], indices.shape: [1,1], params_shape: [100,3,23], slice_dim: 1, and batch_dim: 1 [Op:ResourceScatterNdUpdate] In call to configurable 'SyncOffPolicyDriver' (<function SyncOffPolicyDriver.init at 0x7fb4abb8c2f0>) In call to configurable 'train_eval' (<function train_eval at 0x7fb4abb71d90>)

Is this what you saw? Might be something else affecting it.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/HorizonRobotics/SocialRobot/pull/117?email_source=notifications&email_token=ACKOCGGVLPW5WAYFOEML3ZLRC3J5NA5CNFSM4J4P2ZX2YY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOCVTZPEA#discussion_r379560629, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACKOCGDN5I27YKK52M5KNX3RC3J5NANCNFSM4J4P2ZXQ .