Denys88 / rl_games

RL implementations
MIT License
800 stars 135 forks source link

Normalization implementation is actually computing standardization #281

Open anishhdiwan opened 3 months ago

anishhdiwan commented 3 months ago

Hello! This issue is regarding the "normalization" implemented using RunningMeanStd. Algorithms that use the normalize_input and normalize_value config params use RunningMeanStd internally to keep track of running means and variances and then compute a transformation of $\frac{(x - mean)}{stdev}$. This is however the computation for standardization and not normalization.

For reference, normalization (in range [0,1]) is implemented using $\frac{(x - min)}{(min - max)}$. Raising this issue to either request a distinction in the config or to add a new normalization class.

References

  1. https://stats.stackexchange.com/questions/10289/whats-the-difference-between-normalization-and-standardization
  2. https://www.geeksforgeeks.org/normalization-vs-standardization/
Denys88 commented 3 months ago

hi @anishhdiwan, Yeah you are right it is an old names. Hard to rename some variables because afraid to break configs. I've tested normalization vs standardization. Including normalization using p05 and p95. Standardization worked much better for most of the envs. I think I didn't merge it but I have branch somewhere.

anishhdiwan commented 3 months ago

Hey @Denys88, thanks for the response. I understand that renaming might be a bit messy. But it's nice to know that standardization vs normalization has been tested in the past. Perhaps a comment in the configs (or readme) might be a simple temporary solution to avoid errors in scientific communication (would be nice to make the distinction clear as rl_games is often part of research codebases)?

In any case, thanks again :)