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.

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

fixing object_id_to_object_name as it was breaking the gazing when us… #76

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

Conversation

tusharsangam
Copy link
Contributor

…ing mask-rcnn when the misclassification occurs, also adding the gaze test file in tests folder

…ing mask-rcnn when the misclassification occurs
@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 Oct 6, 2023
Copy link
Contributor

@jimmytyyang jimmytyyang left a comment

Choose a reason for hiding this comment

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

Hi Tushar,

Thank you so much for working on this PR. It looks nice. Please take a look at the comments, and make sure that we run pre-commit to format the code for a PR. If you cannot find the pointer for pre-commit, we can find some time to set it up. Thank you!

spot_rl_experiments/spot_rl/envs/skill_manager.py Outdated Show resolved Hide resolved
spot_rl_experiments/spot_rl/envs/skill_manager.py Outdated Show resolved Hide resolved
spot_rl_experiments/spot_rl/envs/skill_manager.py Outdated Show resolved Hide resolved
spot_rl_experiments/spot_rl/utils/img_publishers.py Outdated Show resolved Hide resolved
Copy link
Contributor

@akshararai akshararai left a comment

Choose a reason for hiding this comment

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

some clean up comments

spot_rl_experiments/spot_rl/envs/base_env.py Outdated Show resolved Hide resolved
spot_rl_experiments/spot_rl/envs/skill_manager.py Outdated Show resolved Hide resolved
spot_rl_experiments/spot_rl/envs/skill_manager.py Outdated Show resolved Hide resolved
spot_rl_experiments/spot_rl/envs/skill_manager.py Outdated Show resolved Hide resolved
spot_rl_experiments/spot_rl/envs/skill_manager.py Outdated Show resolved Hide resolved
spot_rl_experiments/spot_rl/envs/skill_manager.py Outdated Show resolved Hide resolved
spot_rl_experiments/spot_rl/utils/img_publishers.py Outdated Show resolved Hide resolved
tests/hardware_tests/gaze_robustness_test.py Outdated Show resolved Hide resolved
Copy link
Contributor

@jimmytyyang jimmytyyang left a comment

Choose a reason for hiding this comment

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

left the minor comments, overall it looks good!

spot_rl_experiments/spot_rl/envs/skill_manager.py Outdated Show resolved Hide resolved
spot_rl_experiments/spot_rl/utils/img_publishers.py Outdated Show resolved Hide resolved
tests/hardware_tests/gaze_robustness_test.py Outdated Show resolved Hide resolved
@KavitShah1998
Copy link
Contributor

Thank you for working on this @tusharsangam , I have left some comments. Let me know if they make sense.

@tusharsangam
Copy link
Contributor Author

@KavitShah1998 @jimmytyyang Let me know if this looks good to create merge request

@jimmytyyang jimmytyyang self-requested a review October 20, 2023 14:16
Copy link
Contributor

@jimmytyyang jimmytyyang left a comment

Choose a reason for hiding this comment

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

LGTM! please rebase to main before merging!

Comment on lines +1 to +43
from spot_rl.envs.gaze_env import construct_config_for_gaze
from spot_rl.envs.nav_env import construct_config_for_nav
from spot_rl.envs.place_env import construct_config_for_place
from spot_rl.envs.skill_manager import SpotSkillManager
from spot_rl.utils.utils import map_user_input_to_boolean

if __name__ == "__main__":
run_the_loop = True # Don't break the test loop
is_in_position = False # is it before the receptacle
# Construct all three configs
nav_config, pick_config, place_config = (
construct_config_for_nav(),
construct_config_for_gaze(),
construct_config_for_place(),
)
# Modify any values in the config
nav_config.SUCCESS_DISTANCE = 0.10
place_config.SUCCESS_DISTANCE = 0.10
pick_config.SUCCESS_DISTANCE = 0.10
pick_config.MAX_EPISODE_STEPS = 350

while run_the_loop:
# Send the modified configs in SpotSkillManager
spotskillmanager = SpotSkillManager(nav_config, pick_config, place_config)
if not is_in_position:
spotskillmanager.nav("pick_table_05_45")
# Reset Arm before gazing starts
spotskillmanager.get_env().reset_arm()
spotskillmanager.gaze_controller.reset_env_and_policy("ball")
pick_stats = spotskillmanager.pick("ball")
# print(pick_stats)
spotskillmanager.get_env().reset_arm()
spotskillmanager.gaze_controller.reset_env_and_policy("ball")
is_in_position = False
# Navigate to Test Receotacle
spotskillmanager.nav("pick_table_05_45")
is_in_position = True

run_the_loop = map_user_input_to_boolean(
"Do you want to continue to next test or dock & exit ?"
)
if not run_the_loop:
spotskillmanager.dock()
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 move all experimental files in experiments/object_perception_manipulation folder inside spot_rl_experiments/spot_rl/

Also add a read-me there.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
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

5 participants