Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Simplify configs #550

Draft
wants to merge 12 commits into
base: main
Choose a base branch
from
Draft

Simplify configs #550

wants to merge 12 commits into from

Conversation

aliberts
Copy link
Collaborator

@aliberts aliberts commented Dec 5, 2024 •
edited
Loading

What this does

TODO:

  • Create Config classes:
    • TrainPipelineConfig
    • EvalPipelineConfig
    • EvalConfig
    • OfflineConfig
    • OnlineConfig
    • DatasetConfig
    • OptimizerConfig
    • LRSchedulerConfig
    • WandBConfig
  • Link input/output shapes of policies to datasets
    • parse_features_from_dataset
    • parse_features_from_env
  • Allow to either specify an optimizer/scheduler from the training config or use the policy training presets (optimizer and scheduler for now). This adresses the issues discussed in Move function make_optimizer_and_scheduler to policy #401
  • PretrainedConfig
    • ACT
    • Diffusion
    • TDMPC
    • VQBeT
  • Scripts
  • Handle MultiLeRobotDataset
  • Remove hydra completely
  • Update docstrings

How it was tested

  • Reproduce results

How to checkout & try? (for the reviewer)

TODO

@aliberts aliberts self-assigned this Dec 5, 2024
@aliberts aliberts added 🔄 Refactor Refactoring 🔧 Config Change / add / remove configuration labels Dec 5, 2024
dlwh
dlwh reviewed Dec 13, 2024
View reviewed changes
lerobot/configs/default.py Outdated
"takes precedence.",
)
# Use the checkpoint config instead of the provided config (but keep `resume` parameter).
self = checkpoint_cfg
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pretty sure this doesn't do what you want?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indeed that doesn't work at all, I will handle that part once I'm done with the rest of the config (I'm on the policies right now, which is quite a big chunk).
Thanks for the heads up!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dlwh dlwh dlwh left review comments

At least 1 approving review is required to merge this pull request.

Assignees

@aliberts aliberts

Labels
🔧 Config Change / add / remove configuration 🔄 Refactor Refactoring
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@aliberts @dlwh