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

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

Closed
jiaweihhuang opened this issue Apr 13, 2021 · 1 comment
Closed

Comments

@jiaweihhuang
Copy link

jiaweihhuang commented Apr 13, 2021

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!

@jiaweihhuang jiaweihhuang changed the title Can not find implementation of environment encoder of HiP-BMDP HiP-BMDP algorithm seems incomplete Apr 13, 2021
@jiaweihhuang jiaweihhuang reopened this Apr 13, 2021
@jiaweihhuang jiaweihhuang changed the title HiP-BMDP algorithm seems incomplete Some questions related to the implementation of HiP-BMDP algorithm Apr 13, 2021
shagunsodhani added a commit that referenced this issue Apr 22, 2021
@shagunsodhani
Copy link
Contributor

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

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants