b900fdf6f2
Closes #947 This removes all kwargs from all policy constructors. While doing that, I also improved several names and added a whole lot of TODOs. ## Functional changes: 1. Added possibility to pass None as `critic2` and `critic2_optim`. In fact, the default behavior then should cover the absolute majority of cases 2. Added a function called `clone_optimizer` as a temporary measure to support passing `critic2_optim=None` ## Breaking changes: 1. `action_space` is no longer optional. In fact, it already was non-optional, as there was a ValueError in BasePolicy.init. So now several examples were fixed to reflect that 2. `reward_normalization` removed from DDPG and children. It was never allowed to pass it as `True` there, an error would have been raised in `compute_n_step_reward`. Now I removed it from the interface 3. renamed `critic1` and similar to `critic`, in order to have uniform interfaces. Note that the `critic` in DDPG was optional for the sole reason that child classes used `critic1`. I removed this optionality (DDPG can't do anything with `critic=None`) 4. Several renamings of fields (mostly private to public, so backwards compatible) ## Additional changes: 1. Removed type and default declaration from docstring. This kind of duplication is really not necessary 2. Policy constructors are now only called using named arguments, not a fragile mixture of positional and named as before 5. Minor beautifications in typing and code 6. Generally shortened docstrings and made them uniform across all policies (hopefully) ## Comment: With these changes, several problems in tianshou's inheritance hierarchy become more apparent. I tried highlighting them for future work. --------- Co-authored-by: Dominik Jain <d.jain@appliedai.de>
45 lines
1.4 KiB
Python
45 lines
1.4 KiB
Python
from typing import Any
|
|
|
|
from torch import nn
|
|
|
|
from tianshou.data.types import RolloutBatchProtocol
|
|
from tianshou.policy import C51Policy
|
|
from tianshou.utils.net.discrete import NoisyLinear
|
|
|
|
|
|
# TODO: this is a hacky thing interviewing side-effects and a return. Should improve.
|
|
def _sample_noise(model: nn.Module) -> bool:
|
|
"""Sample the random noises of NoisyLinear modules in the model.
|
|
|
|
Returns True if at least one NoisyLinear submodule was found.
|
|
|
|
:param model: a PyTorch module which may have NoisyLinear submodules.
|
|
:returns: True if model has at least one NoisyLinear submodule;
|
|
otherwise, False.
|
|
"""
|
|
sampled_any_noise = False
|
|
for m in model.modules():
|
|
if isinstance(m, NoisyLinear):
|
|
m.sample()
|
|
sampled_any_noise = True
|
|
return sampled_any_noise
|
|
|
|
|
|
# TODO: is this class worth keeping? It barely does anything
|
|
class RainbowPolicy(C51Policy):
|
|
"""Implementation of Rainbow DQN. arXiv:1710.02298.
|
|
|
|
Same parameters as :class:`~tianshou.policy.C51Policy`.
|
|
|
|
.. seealso::
|
|
|
|
Please refer to :class:`~tianshou.policy.C51Policy` for more detailed
|
|
explanation.
|
|
"""
|
|
|
|
def learn(self, batch: RolloutBatchProtocol, *args: Any, **kwargs: Any) -> dict[str, float]:
|
|
_sample_noise(self.model)
|
|
if self._target and _sample_noise(self.model_old):
|
|
self.model_old.train() # so that NoisyLinear takes effect
|
|
return super().learn(batch, **kwargs)
|