Change env configuration, added option for acceleration/steering velocity input #74
Conversation
gym/f110_gym/envs/base_classes.py
Outdated
| self.steer_buffer = np.append(raw_steer, self.steer_buffer) | ||
| else: | ||
| steer = self.steer_buffer[-1] | ||
| self.steer_buffer = self.steer_buffer[:-1] | ||
| self.steer_buffer = np.append(raw_steer, self.steer_buffer) | ||
|
|
||
| if self.control_input == "speed": |
There was a problem hiding this comment.
What about creating an action abstraction that process the given action and returns acceleration and steering velocity?
This would allow:
- a cleaner extension to more actions (e.g., discrete, meta-action, etc.), instead of starting nesting if-then-else.
- better separation between action processing and update_pose
For example, assuming update_pose is only responsible for stepping the dynamics model given acceleration and steering velocity, we could change line 640 as:
# looping over agents
for i, agent in enumerate(self.agents):
# update each agent's pose
accl, sv = self.action_type.act(action) # action_type implements the conversion to accl, sv
current_scan = agent.update_pose(accl, sv)
agent_scans.append(current_scan)
There was a problem hiding this comment.
Added classes for action types and specific implementations when they take actions to accl and sv.
I kept the accl and sv calculation at where it is right now, and will change that in a new pr with the update_pose changes.
There was a problem hiding this comment.
Ok, the implementation looks good.
Imo base_classes.py contains too many things. I moved it into a separate file action.py
@hzheng40 check if it is ok please
update string mapping method Co-authored-by: Luigi Berducci <luigi.berd@gmail.com>
update string mapping method Co-authored-by: Luigi Berducci <luigi.berd@gmail.com>
update string mapping method Co-authored-by: Luigi Berducci <luigi.berd@gmail.com>
|
Updated, @luigiberducci please check again. |
move deep_update in utils.py, update f110_env.py accordingly
add test to check params update is propagated to simulator and racecars
rename gym_api_test.py to f110_env_test.py, move test configure() in f110_env_test.py, refactoring utility test
There was a problem hiding this comment.
Changes:
- The implementation has been separated into
utils.pyandaction.py. - The bug of the
configuremethod has been fixed. - Added tests for utils and configure.
- The old tests have been adapted to the new signature of the env constructor.
- Renamed test file from
gym_api_test.pytof110_env_test.py
LGTM
gym/f110_gym/envs/base_classes.py
Outdated
| self.steer_buffer = np.append(raw_steer, self.steer_buffer) | ||
| else: | ||
| steer = self.steer_buffer[-1] | ||
| self.steer_buffer = self.steer_buffer[:-1] | ||
| self.steer_buffer = np.append(raw_steer, self.steer_buffer) | ||
|
|
||
| if self.control_input == "speed": |
There was a problem hiding this comment.
Ok, the implementation looks good.
Imo base_classes.py contains too many things. I moved it into a separate file action.py
@hzheng40 check if it is ok please
|
Looks good, thanks! Merging. |
Fixes #72 #73
Also linting (sorry)