TD3: fixed dimension of clipped_noise for target actions, added noise …

[dosssman]

Description

Closes #279.

• td3_continuous_action.py: noise sampled to compute the Q network update target matches action dimensions in the buffer
• td3_continuous_action.py: aforementioned noise is also scaled to match the scaling range of the actions.

Types of changes

• [x] Bug fix
• [ ] New feature
• [ ] New algorithm
• [ ] Documentation

Checklist:

• [x] I've read the CONTRIBUTION guide (required).
• [x] I have ensured pre-commit run --all-files passes (required).
• [ ] I have updated the documentation and previewed the changes via mkdocs serve.
• [x] I have updated the tests accordingly (if applicable).

If you are adding new algorithms or your change could result in performance difference, you may need to (re-)run tracked experiments. See https://github.com/vwxyzjn/cleanrl/pull/137 as an example PR.

• [x] I have contacted vwxyzjn to obtain access to the openrlbenchmark W&B team (required).
• [x] I have tracked applicable experiments in openrlbenchmark/cleanrl with --capture-video flag toggled on (required).
• [x] I have added additional documentation and previewed the changes via mkdocs serve.
  • [x] I have explained note-worthy implementation details.
  • [x] I have explained the logged metrics.
  • [x] I have added links to the original paper and related papers (if applicable).
  • [x] I have added links to the PR related to the algorithm.
  • [x] I have created a table comparing my results against those from reputable sources (i.e., the original paper or other reference implementation).
  • [x] I have added the learning curves (in PNG format with width=500 and height=300).
  • [x] I have added links to the tracked experiments.
  • [x] I have updated the overview sections at the docs and the repo
• [x] I have updated the tests accordingly (if applicable).

[vercel[bot]]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Updated
cleanrl	✅ Ready (Inspect)	Visit Preview	Oct 19, 2022 at 6:46PM (UTC)

[dosssman]

@vwxyzjn td3 continuous JAX variant will probably be affected too though.

[vwxyzjn]

@joaogui1 could you check if TD3 (JAX) will be affected?

[joaogui1]

@vwxyzjn check whether the jax implementation affected by this PR or if it will also need a fix like this PR?

[vwxyzjn]

@joaogui1, the latter :)

[joaogui1]

Got it, it will need to be fix, creating the PR this moment

[vwxyzjn]

@dosssman thank you for the PR! Would you mind running some benchmark experiments to see if this change has a significant impact on the performance? If not, we don't even have to update the docs, since the main purpose of re-doing benchmark is to ensure no regression in performance.

https://github.com/vwxyzjn/cleanrl/blob/f0bbf49ff2f0786882dc8ac0eded8b483a36b6ca/benchmark/td3.sh#L3-L7

[dosssman]

There is a performance regression on the Walker2d env, but the others are only marginally affected:

Report here

[vwxyzjn]

Hmm interesting. Thanks for running the expriments. Although one experiment failed... Would you mind re-running it?

[vwxyzjn]

Given that this is a performance-impacting change, I am re-running the benchmark now.

[vwxyzjn]

No material change to the performance (there is a minor regression in Walker2d-v2. I am going to update the docs and merge the PR. The regression report is at https://wandb.ai/openrlbenchmark/cleanrl-cache/reports/-281-CleanRL-s-TD3-TD3-Trgt-Action-Noise-Sample-Check--VmlldzoyODE5MTg0