Skip to content

Small fix to rllib visualizer #338

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

Closed
wants to merge 1 commit into from

Conversation

cathywu
Copy link
Member

@cathywu cathywu commented Dec 25, 2018

Checkpoint files (from RLlib experiments) used to be written to ~/ray_results/experiment_tag/Scenario/checkpoint_N/checkpoint_N.something, but now they seem to be written to ~/ray_results/experiment_tag/Scenario/checkpoint_N.something. This PR change reflects that change in directory structure, for when we try to visualize from the checkpoint.

@eugenevinitsky @fywu85 @AboudyKreidieh Could you confirm this?

@cathywu
Copy link
Member Author

cathywu commented Dec 25, 2018

@eugenevinitsky @AboudyKreidieh I see that the Travis build is failing. Could there be a ray version difference in the Travis build?

Here is what a sample results directory looks like to me (no checkpoint_N subdirectories) with a clean install done today. But, the unit tests corresponding to the checkpoint locations are failing on Travis.

screen shot 2018-12-25 at 12 35 52 am

@AboudyKreidieh
Copy link
Member

@cathywu I'm not sure about running these locally (haven't run rllib experiments since the new changes), but if you check in the data folder in tests they are under the subdirectory checkpoint_N, hence the failing tests.

@cathywu
Copy link
Member Author

cathywu commented Dec 25, 2018 via email

@cathywu
Copy link
Member Author

cathywu commented Dec 26, 2018

Update: I was mistaken above, the Travis builds are dynamically pulling from flow-project/ray, but from the ray_merge branch and not master. This discrepancy is the cause of the issue.

This PR was issued in accordance to incompatibility of the rllib_visualizer with flow-project/ray:master. Once flow-project/ray#2 is addressed, we can close this PR (without merge).

@eugenevinitsky
Copy link
Member

Yeah, a fix for this is coming. We're behind Ray and should try to upgrade to 0.6.1

@AboudyKreidieh
Copy link
Member

we've upgraded to ray 0.6.1 since, so I am assuming this is now resolved. If I am wrong, please feel free to reopen.

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

Successfully merging this pull request may close these issues.

3 participants