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

ObsUtils.unprocess_obs_dict() modifies obs dict in-place #41

Closed
MaxDu17 opened this issue Aug 11, 2022 · 5 comments
Closed

ObsUtils.unprocess_obs_dict() modifies obs dict in-place #41

MaxDu17 opened this issue Aug 11, 2022 · 5 comments

Comments

@MaxDu17
Copy link

MaxDu17 commented Aug 11, 2022

The ObsUtils.unprocess_obs_dict() seems to modify the observation dictionary that is passed in, in addition to returning it. For example, I observed that in the lines referenced below, next_obs images are between 0 and 1, while after line 147, next_obs images are between 0 and 255. This leads to a problem, as obs is derived from next_obs, which means that in the next loop around, we will pass already unprocessed images into unprocess_obs_dict(). This has led to some significant issues, as the saved images are corrupted from what is observed. As a simple fix, I wrapped the next_obs in deepcopy(next_obs) on line 147.

if return_obs:
# Note: We need to "unprocess" the observations to prepare to write them to dataset.
# This includes operations like channel swapping and float to uint8 conversion
# for saving disk space.
traj["obs"].append(ObsUtils.unprocess_obs_dict(obs))
traj["next_obs"].append(ObsUtils.unprocess_obs_dict(next_obs))

@amandlek
Copy link
Member

Great catch - thanks for reporting this bug! We will PR a fix soon.

@MaxDu17
Copy link
Author

MaxDu17 commented Aug 11, 2022

Awesome! Thanks.

@MaxDu17 MaxDu17 closed this as completed Aug 11, 2022
@amandlek amandlek reopened this Aug 11, 2022
@amandlek
Copy link
Member

amandlek commented Aug 11, 2022

Would you be able to send over a trained model on one of our datasets so that we can reproduce this issue easily? Or even confirm that it breaks with one of our models (see here)

@MaxDu17
Copy link
Author

MaxDu17 commented Aug 11, 2022

Does this work? https://drive.google.com/file/d/1QyN5YyZYhzJRAUcTPmRGmebkpyh_9-hy/view?usp=sharing
It's from the can pick-place environment. I just ran the run_trained_agent.py script using this checkpoint from the repository, and the problem is there

@amandlek
Copy link
Member

amandlek commented Jul 6, 2023

Fixed in #71

@amandlek amandlek closed this as completed Jul 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants