facebookresearch / mtrl

Multi Task RL Baselines
MIT License
223 stars 28 forks source link

Some questions related to the implementation of HiP-BMDP algorithm #4

Closed jiaweihhuang closed 3 years ago

jiaweihhuang commented 3 years ago

Description

Hi, it's not an issue about running the code, but some possible typos in the implementation of HiP-BMDP. (so I just ignore the format...)

(1) About the configuration file ./config/agent/components/hipbmdp_multitask.yaml

The default setting is should_use_task_encoder: False. I wonder that, if we want to include environment encoding as a part of the input of the transition model (as Figure 2 in the paper), shall we set it to be True?

Besides, I tried to run the code after setting it to be true, to avoid dimension mismatch error when running the code, I also need to set should_condition_encoder_on_task_info and should_concatenate_task_info_with_encoder to be True in that file. I would like to know that if my current setting is correct for HiP-BMDP algorithm.

(2) maybe a typo in ./mtrl/agent/hipbmdp.py

In line 110, the second argument of function get_task_encoding() is mode, but when this function is called in ./mtrl/agent/sac.py Line 200, the second argument is specified as modes, and it will cause an error when running the code. I guess it's just a typo and I can run the code after fixing it.

Could you help to fix these issues, or please let me know if I misunderstand something? Thanks!

shagunsodhani commented 3 years ago

Thanks for reporting! Yes you are right about both the points. I have added a PR to fix them!