-
Notifications
You must be signed in to change notification settings - Fork 4.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Action buffer #4612
Action buffer #4612
Changes from 76 commits
ba8bdcf
6c93137
03f7e47
87d2049
a889e8f
24be76f
12fa45a
ff4f3b8
3e807c6
fed5c20
d119c1a
5a37dfe
1e5e440
194505e
4baaa7a
60337b8
841110a
ebd50b2
2e4dcf2
1337d07
c05c40e
1b96170
c940d41
b0d9a48
d2bb5d0
ad144c3
9090821
f23e395
785848e
9af9ee9
600d307
42bdfce
754f5b8
a8813fc
64091cc
a8204bd
b5ca548
ed11b10
442f29a
8733ec1
199d15b
00a824c
b0ed241
bfaa249
d927497
0d33e1f
8f06a67
da1c85a
080f3eb
f872359
5886f74
fe8fdd9
9479a65
3a90973
dbf819c
d1e2b97
e87effe
e0418dc
5f571a1
9089e63
f8d85fa
c21d223
f0f4249
d6eaf8d
e9848b1
b25fc3d
10944f1
6fcdd3f
6d4738b
5c8ec2d
0441118
86b6d71
2bf004c
aaf6c59
056cf6d
5691f60
b567fcd
116580a
b152511
bb9988c
c488e8e
589907a
c8ae8da
c651ebc
0dc4396
434f210
714b444
65d17fe
4fc60d5
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -18,6 +18,7 @@ | |
DecisionSteps, | ||
TerminalSteps, | ||
BehaviorSpec, | ||
ActionTuple, | ||
BehaviorName, | ||
AgentId, | ||
BehaviorMapping, | ||
|
@@ -236,7 +237,7 @@ def __init__( | |
|
||
self._env_state: Dict[str, Tuple[DecisionSteps, TerminalSteps]] = {} | ||
self._env_specs: Dict[str, BehaviorSpec] = {} | ||
self._env_actions: Dict[str, np.ndarray] = {} | ||
self._env_actions: Dict[str, ActionTuple] = {} | ||
self._is_first_message = True | ||
self._update_behavior_specs(aca_output) | ||
|
||
|
@@ -336,7 +337,7 @@ def _assert_behavior_exists(self, behavior_name: str) -> None: | |
f"agent group in the environment" | ||
) | ||
|
||
def set_actions(self, behavior_name: BehaviorName, action: np.ndarray) -> None: | ||
def set_actions(self, behavior_name: BehaviorName, action: ActionTuple) -> None: | ||
self._assert_behavior_exists(behavior_name) | ||
if behavior_name not in self._env_state: | ||
return | ||
|
@@ -346,7 +347,7 @@ def set_actions(self, behavior_name: BehaviorName, action: np.ndarray) -> None: | |
self._env_actions[behavior_name] = action | ||
|
||
def set_action_for_agent( | ||
andrewcoh marked this conversation as resolved.
Show resolved
Hide resolved
|
||
self, behavior_name: BehaviorName, agent_id: AgentId, action: np.ndarray | ||
self, behavior_name: BehaviorName, agent_id: AgentId, action: ActionTuple | ||
) -> None: | ||
self._assert_behavior_exists(behavior_name) | ||
if behavior_name not in self._env_state: | ||
|
@@ -366,7 +367,10 @@ def set_action_for_agent( | |
agent_id | ||
) | ||
) from ie | ||
self._env_actions[behavior_name][index] = action | ||
if action_spec.continuous_size > 0: | ||
self._env_actions[behavior_name].continuous[index] = action.continuous[0, :] | ||
if action_spec.discrete_size > 0: | ||
self._env_actions[behavior_name].discrete[index] = action.discrete[0, :] | ||
|
||
def get_steps( | ||
self, behavior_name: BehaviorName | ||
|
@@ -410,15 +414,20 @@ def _close(self, timeout: Optional[int] = None) -> None: | |
|
||
@timed | ||
def _generate_step_input( | ||
self, vector_action: Dict[str, np.ndarray] | ||
self, vector_action: Dict[str, ActionTuple] | ||
) -> UnityInputProto: | ||
rl_in = UnityRLInputProto() | ||
for b in vector_action: | ||
n_agents = len(self._env_state[b][0]) | ||
if n_agents == 0: | ||
continue | ||
for i in range(n_agents): | ||
action = AgentActionProto(vector_actions=vector_action[b][i]) | ||
# TODO: extend to AgentBuffers | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What does this TODO mean? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Meant as a TODO for the C# changes that change the proto to accept both continuous and discrete. Poorly worded on my part. |
||
if vector_action[b].continuous is not None: | ||
_act = vector_action[b].continuous[i] | ||
else: | ||
_act = vector_action[b].discrete[i] | ||
action = AgentActionProto(vector_actions=_act) | ||
rl_in.agent_actions[b].value.extend([action]) | ||
rl_in.command = STEP | ||
rl_in.side_channel = bytes( | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -97,11 +97,6 @@ def test_step(mock_communicator, mock_launcher): | |
env.step() | ||
with pytest.raises(UnityActionException): | ||
env.set_actions("RealFakeBrain", spec.action_spec.empty_action(n_agents - 1)) | ||
decision_steps, terminal_steps = env.get_steps("RealFakeBrain") | ||
n_agents = len(decision_steps) | ||
env.set_actions("RealFakeBrain", spec.action_spec.empty_action(n_agents) - 1) | ||
env.step() | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why was this piece of test removed ? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. bump |
||
env.close() | ||
assert isinstance(decision_steps, DecisionSteps) | ||
assert isinstance(terminal_steps, TerminalSteps) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need some constructor that will take only continuous or only discrete so the user does not have to create an empty array when using only discrete or only continuous.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why can't the default be
None
but in the constructor assigns an empty array when None is specified? This is a common pattern for mutable default parameters