allenai / allenact

An open source framework for research in Embodied-AI from AI2.
https://www.allenact.org
Other
313 stars 50 forks source link

Update Manipulathor plugin #320

Closed twni2016 closed 2 years ago

twni2016 commented 2 years ago

This PR focuses on manipulathor plugin only, including several changes:

lgtm-com[bot] commented 2 years ago

This pull request introduces 14 alerts when merging 59a2f210ed72db1a879d616ffaf2d62f4c80945e into 894c2ce1b7d89402419474829a6f308d225bec6f - view on LGTM.com

new alerts:

lgtm-com[bot] commented 2 years ago

This pull request introduces 3 alerts when merging 59a2f210ed72db1a879d616ffaf2d62f4c80945e into 4a4fd5f33b64311f09c9c0947fbe59b40ced3d0c - view on LGTM.com

new alerts:

lgtm-com[bot] commented 2 years ago

This pull request introduces 3 alerts when merging d1509098800ce0f34664235123d165894f37173e into 4a4fd5f33b64311f09c9c0947fbe59b40ced3d0c - view on LGTM.com

new alerts:

twni2016 commented 2 years ago

Most of comments have been resolved. Now I move the vibrations.csv into my repo and defined it in my repo too.

Remaining issue: https://github.com/allenai/allenact/pull/320#discussion_r765133533 where to place the csv file.

lgtm-com[bot] commented 2 years ago

This pull request introduces 3 alerts when merging 0474ab8badb951e521dd89123121218ba44743bc into fee80031469a89967803bfeeabc7a7b22764faf1 - view on LGTM.com

new alerts:

lgtm-com[bot] commented 2 years ago

This pull request introduces 3 alerts when merging 795c1d891a3bf4d974454df115b91ab38b7c2699 into 8479e03ec627cce9c84bc7c7582fdedaaeca03df - view on LGTM.com

new alerts:

twni2016 commented 2 years ago

Looks good to me! I think we may want to refactor how we use _vibration_dist_dict in the future (to make this even easier for people to get automatically) but it seems fine as it is now.

Yes, I upload the script to generate the csv file into my repo: please check https://github.com/allenai/disturb-free/tree/main/manipulathor_plugin

twni2016 commented 2 years ago

@Lucaweihs I think this PR has been almost completed. The rest issues in LGTM are minor I think.

lgtm-com[bot] commented 2 years ago

This pull request introduces 2 alerts when merging e16dcd16b0715172e9a6e907756dade82fe0cce7 into a785552faefce6537a74ae2b653c8496f8258736 - view on LGTM.com

new alerts:

lgtm-com[bot] commented 2 years ago

This pull request introduces 2 alerts when merging e16dcd16b0715172e9a6e907756dade82fe0cce7 into 348a30d76e2cbe31c3f26ed11df3d1f605deeee0 - view on LGTM.com

new alerts: