Simba SAC

[araffin]

Description

Motivation and Context

• I have raised an issue to propose this change (required for new features and bug fixes)

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)
• Documentation (update in the documentation)

Checklist:

• I've read the CONTRIBUTION guide (required)
• I have updated the changelog accordingly (required).
• My change requires a change to the documentation.
• I have updated the tests accordingly (required for a bug fix or a new feature).
• I have updated the documentation accordingly.
• I have reformatted the code using make format (required)
• I have checked the codestyle using make check-codestyle and make lint (required)
• I have ensured make pytest and make type both pass. (required)
• I have checked that the documentation builds using make doc (required)

Note: You can run most of the checks using make commit-checks.

Note: we are using a maximum length of 127 characters per line

[LucasAlegre]

Hi @araffin! I was curious to see that you have not implemented RSNorm (at least until now) in this Simba implementation. From the paper (see Figure 12), RSNorm is critical to the performance of Simba. I found this particularly very surprising, and I was wondering why not simply using BatchNorm to normalize the inputs has the same effect (from Figure 12, it is much worse).

[araffin]

I was curious to see that you have not implemented RSNorm (at least until now) in this Simba implementation.

because I use VecNormalize:

import optax


default_hyperparams = dict(
    n_envs=1,
    n_timesteps=int(5e5),
    policy="SimbaPolicy",
    learning_rate=3e-4,
    # qf_learning_rate=1e-3,
    policy_kwargs={
        "optimizer_class": optax.adamw,
        # "optimizer_kwargs": {"weight_decay": 0.01},
        "net_arch": {"pi": [128], "qf": [256, 256]},
        "n_critics": 2,
    },
    learning_starts=10_000,
    normalize={"norm_obs": True, "norm_reward": False},
)

hyperparams = {}

for env_id in [
    "HalfCheetah-v4",
    "Humanoid-v4",
    "HalfCheetahBulletEnv-v0",
    "Ant-v4",
    "Hopper-v4",
    "Walker2d-v4",
    "Swimmer-v4",
    "AntBulletEnv-v0",
    "HopperBulletEnv-v0",
    "Walker2DBulletEnv-v0",
    "BipedalWalkerHardcore-v3",
    "Pendulum-v1",
]:
    hyperparams[env_id] = default_hyperparams

So far in my test, having a second critic was more important. I'm suspecting that the hyperparameters presented are overfitted to the dmc hard benchmark.

why not simply using BatchNorm

probably because they would need to use CrossQ for that.

[LucasAlegre]

That is interesting, I was also surprised they removed clipped double q-learning from SAC, and there is no ablation on that in the paper. At the moment, I am using CrossQ+DroQ for a personal project, and I am really curious if its worth changing it to Simba. It would be really cool if you could share your findings, thanks! :)