Skip to content
This repository has been archived by the owner on Sep 1, 2024. It is now read-only.

Migration to hydra latest #187

Open
wants to merge 8 commits into
base: main
Choose a base branch
from

Conversation

RedLeader962
Copy link

@RedLeader962 RedLeader962 commented Jul 12, 2024

Types of changes

  • Docs change / refactoring / dependency upgrade
  • 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)

Motivation and Context / Related issue

Motivation: Enable importing mbrl-lib in project requiring newer hydra-core>=1.1.2 version.
Context: mbrl-lib hydra-core version was freezed at v1.0.3 which was release in 2020. A lot of useful functionality were introduce since then.

Regression warning/bug that where fixed:
Post hydra-core version 1.1 introduce breaking changes that were fixed in this PR the following way:

Remark on breaking change:

  • overriding via command line now require prefixing with ++ eg: python -m mbrl.examples.main algorithm=mbpo ++device=cpu

Note:

  • I've set the minimal requirement to hydra-core>=1.1.2. Older than that wont be compatible with the proposed changes.
  • Documentation was updated

How Has This Been Tested (if it applies)

  • Repeated all the following test with hydra-core==[1.1.2, 1.2, 1.3.2]
    • Executed pytest tests on both tests/core and tests/algorithms
    • Manually run the modified example at mbrl/examples/README.md
      python -m mbrl.examples.main \
         algorithm=mbpo \
        overrides=mbpo_cartpole \
        dynamics_model=gaussian_mlp_ensemble \
        ++device=cpu \
        ++overrides.sac_batch_size=256 \
        ++overrides.validation_ratio=0.2 \
        ++dynamics_model.activation_fn_cfg._target_=torch.nn.ReLU 
  • Execute same command as in .github/workflows/ci.yml
    • Execute dependencies install via requirement file
    • Lint with flake8
    • Lint with mypy
    • Check format with black

Checklist

  • The documentation is up-to-date with the changes I made.
  • I have read the CONTRIBUTING document and completed the CLA (see CONTRIBUTING).
  • All tests passed, and additional code has been covered with new tests.

…re hydra-core=1.1.0 behaviour.

- relax omegaconf requirement
- bump hydra-core to version 1.1
- Error msg eg: `omegaconf.errors.ValidationError: Error instantiating 'mbrl.models.gaussian_mlp.GaussianMLP' : Object of unsupported type: 'SiLU'`. Note: those error msg for migrating hydra-core 1.o to 1.1 are not really helpful until you open a debug session
- Problem: `hydra-core>=1.1.0` set `instantiate` fct to recursive by default, breaking all mbrl-lib use of `hydra.instantiate` original behaviour with nested dict config ([hydra 1.1.0 release note](https://github.com/facebookresearch/hydra/releases/tag/v1.1.0)).
- Solution: changing config files on the user side would be error-prone, so simply add `_recursive_=False` to every `hydra.utils.instantiate(cfg, _recursive_=False)` that deal with nested instantiation logic.
- `hydra-core<1.1.2` introduced breaking change relevant to mbrl-lib.
- `mbrl-lib` was tested with both `tests/core` and `tests/algorythm` against hydra-core versions 1.1, 1.2 and 1.3
- Preserve mbrl-lib default config ordering behaviour using `_self_` keyword  (Ref https://hydra.cc/docs/1.1/upgrades/1.0_to_1.1/default_composition_order/)
- remove deprecated package header `@package _group_` (ref https://hydra.cc/docs/1.1/upgrades/1.0_to_1.1/changes_to_package_header/)
@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Jul 12, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants