Skip to content
This repository has been archived by the owner on Oct 31, 2023. It is now read-only.

Simple modular configuration refactoring #2

Merged
merged 4 commits into from Sep 7, 2021

Conversation

Aladoro
Copy link
Contributor

@Aladoro Aladoro commented Sep 4, 2021

Refactored the hydra configurations to be divided into agent-specific (agent_cfg) and environment-specific (env_cfg), to ease experimentation. In particular, the only argument that needs to be overridden for any of the experiments with DrQ-v2 is env_cfg.

I have also added the relative configurations for all medium benchmark tasks, following the details from the paper.

@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 Sep 4, 2021
Copy link
Contributor

@denisyarats denisyarats left a comment

Choose a reason for hiding this comment

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

Thanks for submitting this!

I think it is way too complicated and convoluted at the moment. I wonder if there is a way to simplify it. Specifically, it would be nice if we still have the default config as before, but only have a folder with task specific overrides. Right now there is a lot of duplication going on and only a few parameters are changing, it would be best if we can preserve this simplicity.

README.md Outdated
@@ -72,7 +72,7 @@ conda activate drqv2

Train the agent:
```sh
python train.py task=quadruped_walk
python train.py env_cfg=quadruped_walk
Copy link
Contributor

Choose a reason for hiding this comment

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

let's rename it to just task

@@ -0,0 +1,20 @@
# task settings
task: acrobot_swingup
frame_stack: 3
Copy link
Contributor

Choose a reason for hiding this comment

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

hm there is a lot of overlap going on, only a few arguments are changing. I wonder if there is a way to only have a task specific overrides and still preserve the default config.

…the relative configuration files and the parsing logic
@Aladoro
Copy link
Contributor Author

Aladoro commented Sep 7, 2021

As suggested, I changed the config logic to keep the default config as before and have an additional folder with task specific overrides. Also I changed the name of the folder to "task" (and the parameter specifying the task to "task_name" to avoid conflict).

Let me know if this works, or if you had anything else in mind ^^

Copy link
Contributor

@denisyarats denisyarats left a comment

Choose a reason for hiding this comment

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

this looks good now, thanks!

I think the humanoid tasks are missing (maybe some others too), could you please add those as well? otherwise looks great.

@Aladoro
Copy link
Contributor Author

Aladoro commented Sep 7, 2021

Done, added configurations for all easy and hard benchmark tasks (before I included only the medium benchmark tasks)! Let me know if any other change is required ^^

@denisyarats
Copy link
Contributor

thanks, this looks good. will merge now.

@denisyarats denisyarats closed this Sep 7, 2021
@denisyarats denisyarats reopened this Sep 7, 2021
@denisyarats denisyarats merged commit c2eab01 into facebookresearch:main Sep 7, 2021
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.

None yet

3 participants