[RLlib] Quick-fix for default RLModules in combination with a user-provided config-sub-dict (instead of a full DefaultModelConfig).

[sven1977]

Quick-fix for default RLModules in combination with a user-provided config-sub-dict (instead of a full DefaultModelConfig).

Why are these changes needed?

Related issue number

Checks

• I've signed off every commit(by using the -s flag, i.e., git commit -s) in this PR.
• I've run scripts/format.sh to lint the changes in this PR.
• I've included any doc changes needed for https://docs.ray.io/en/master/.
  • I've added any new APIs to the API Reference. For example, if I added a
    method in Tune, I've added it in doc/source/tune/api/ under the
    corresponding .rst file.
• I've made sure the tests are passing. Note that there might be a few flaky tests, see the recent failures at https://flakey-tests.ray.io/
• Testing Strategy
  • Unit tests
  • Release tests
  • This PR is not tested :(

[simonsays1980]

LGTM. Some general questions and maybe we need to add this also to BCRLModule

[simonsays1980]

Does this mean that the user needs to do this everytime she defines a new module? We do here actually the same we did before with the AlgorithmConfig.model_config property.

[sven1977]

Does this mean that the user needs to do this everytime she defines a new module? We do here actually the same we did before with the AlgorithmConfig.model_config property

Nope, this is only(!) needed for RLlib's default models. See my other answer here. Users are completely free to request any keys they need in their custom model classes and then - of course - have to provide these keys in the model_config dict they pass into the RLModuleSpec. Users can also decide to use default values inside their RLModule classes for certain keys.

[simonsays1980]

I guess we need to add this also to the BCRLModule then.

[simonsays1980]

Also: We might need to add this to all the examples that are not derived from the algorithm modules.

[sven1977]

Great catch! Done ...

[sven1977]

For the non-default models, this is fine, b/c those are 100% user-driven and thus don't use the catalogs. The user decides, which keys are accessed in the model_config and only those need to be provided (or even have default values inside the custom RLModule code).

This is really only a problem for RLlib's default models.