Skip to content
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

Using dist.mode instead of logits.argmax. More compact. #1066

Merged
merged 5 commits into from
Mar 2, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion tianshou/policy/modelfree/discrete_sac.py
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ def forward( # type: ignore
logits, hidden = self.actor(batch.obs, state=state, info=batch.info)
dist = Categorical(logits=logits)
if self.deterministic_eval and not self.training:
MischaPanch marked this conversation as resolved.
Show resolved Hide resolved
act = logits.argmax(axis=-1)
act = dist.mode
else:
act = dist.sample()
return Batch(logits=logits, act=act, state=hidden, dist=dist)
Expand Down
15 changes: 1 addition & 14 deletions tianshou/policy/modelfree/pg.py
Original file line number Diff line number Diff line change
Expand Up @@ -158,19 +158,6 @@ def process_fn(
batch: BatchWithReturnsProtocol
return batch

def _get_deterministic_action(self, logits: torch.Tensor) -> torch.Tensor:
if self.action_type == "discrete":
return logits.argmax(-1)
if self.action_type == "continuous":
# assume that the mode of the distribution is the first element
# of the actor's output (the "logits")
return logits[0]
raise RuntimeError(
f"Unknown action type: {self.action_type}. "
f"This should not happen and might be a bug."
f"Supported action types are: 'discrete' and 'continuous'.",
)

def forward(
self,
batch: ObsBatchProtocol,
Expand Down Expand Up @@ -198,7 +185,7 @@ def forward(

# in this case, the dist is unused!
if self.deterministic_eval and not self.training:
act = self._get_deterministic_action(logits)
act = dist.mode
else:
act = dist.sample()
result = Batch(logits=logits, act=act, state=hidden, dist=dist)
Expand Down
5 changes: 4 additions & 1 deletion tianshou/policy/modelfree/redq.py
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,10 @@ def forward( # type: ignore
loc_scale, h = self.actor(batch.obs, state=state, info=batch.info)
loc, scale = loc_scale
dist = Independent(Normal(loc, scale), 1)
act = loc if self.deterministic_eval and not self.training else dist.rsample()
MischaPanch marked this conversation as resolved.
Show resolved Hide resolved
if self.deterministic_eval and not self.training:
act = dist.mode
else:
act = dist.rsample()
log_prob = dist.log_prob(act).unsqueeze(-1)
# apply correction for Tanh squashing when computing logprob from Gaussian
# You can check out the original SAC paper (arXiv 1801.01290): Eq 21.
Expand Down
2 changes: 1 addition & 1 deletion tianshou/policy/modelfree/sac.py
Original file line number Diff line number Diff line change
Expand Up @@ -177,7 +177,7 @@ def forward( # type: ignore
assert isinstance(logits, tuple)
dist = Independent(Normal(*logits), 1)
if self.deterministic_eval and not self.training:
act = logits[0]
act = dist.mode
else:
act = dist.rsample()
log_prob = dist.log_prob(act).unsqueeze(-1)
Expand Down
Loading