Allow python script for hyperparameter configuration

[ernestum]

Description

The yaml files with the hyperparameters are replaced by python files.
The python files must contain a dictionary called hyperparams with a key for each environment mapping to hyperparameter configurations.

Motivation and Context

The yaml file format had severe limitations in terms of expressiveness such that we already were often resorting to evaluating strings in the yaml file. This lead to problems when the referenced classes ware not imported before.
Just loading the configuration as a dictionary from a python file (in which the appropriate includes can just be added at the top) makes all those hacks obsolete.
We could then also remove the --gym-packages parameter!

Would fix #316 , would advance #299 and maybe make #315 obsolete.

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)

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

[araffin]

Hello,
sorry for not being clear enough.
I don't think we should replace yaml per python, but we should allow python file in case more customization is needed.

EDIT: my main motivation for keeping yaml files is that they are easy to read (no quotes everywhere) and it's easy to extend hyperparameters that work for some env.

[ernestum]

Is that the only motivation? Because that we could easily do with python dicts.

Eg use the kwargs notation to create the config dicts like this:

hyperparams = {
    "atari": dict(
        env_wrapper=['stable_baselines3.common.atari_wrappers.AtariWrapper'], 
        frame_stack=4,
        policy="CnnPolicy",
        n_envs=16,
        n_timesteps=10000000.0,
        ent_coef=0.01,
        vf_coef=0.25,
        policy_kwargs=dict(optimizer_class=RMSpropTFLike, optimizer_kwargs=dict(eps=1e-05))
    )
}

and we can easily extend/inherit configs like this:

mujoco_config = dict(...)

halfcheetah = mujoco_config.copy()
halfcheetah.update(
    vf_coef = 0.5
)

If you want to stay backwards compatible that is fine but if not then I would prefer minimizing the codebase instead.
What do you think @araffin ?

[araffin]

Is that the only motivation? Because that we could easily do with python dicts.

It is true that you can have it a bit cleaner, but it still comes with some overhead notations and can be parsed with python script only.

If you want to stay backwards compatible

This is also another important motivation.

I would prefer minimizing the codebase instead.

I think it should be pretty minimal to support both, no?
Re-using https://github.com/DLR-RM/rl-baselines3-zoo/blob/master/rl_zoo3/exp_manager.py#L387-L390

[ernestum]

You are right. It should not be too much work/extra code.

[araffin]

You are right. It should not be too much work/extra code.

btw, do you have a convert script? or did you do the conversion by hand?

EDIT: for now, I would only put an example python script and keep the right as-is

[ernestum]

I only half-automated this with the interactive console and then did some manual tweaks (turning the strings to evaluate into dicts).
Yes I will revert the config files back to the old yaml format.

[ernestum]

Hmm I can't replicate the flake8 error in the pipeline locally. @araffin do you have an idea what this could be?

[araffin]

yep, not your fault... https://stackoverflow.com/questions/74558565/flake8-error-code-supplied-to-ignore-option-does-not-match-a-z1-30

A new version of flake8 was released https://flake8.pycqa.org/en/latest/release-notes/6.0.0.html

PyCQA/flake8#1760

[araffin]

I hope that my PR for flake8 will get merged too https://github.com/PyCQA/flake8/compare/main...araffin:flake8:fix/improve-error-message?expand=1 (but currently all external contribution is prohibited)

[araffin]

Please update the README for documentation and we are good to go ;)

[araffin]

We could then also remove the --gym-packages parameter!

it does work because the saved yaml file contains the correct imports?
(I think it may still fail if the env is not registered, so --gym-packages might still be needed)

[ernestum]

it does work because the saved yaml file contains the correct imports?

Yes I think so. Whatever you would put into rl_zoo3/import_envs.py you can now just put straight into the config file.

[araffin]

LGTM, thanks =)