Skip to content
This repository was archived by the owner on May 21, 2022. It is now read-only.

Add close function, remove close kw in render#26

Merged
JobJob merged 2 commits intoJuliaML:masterfrom
albheim:master
Feb 13, 2019
Merged

Add close function, remove close kw in render#26
JobJob merged 2 commits intoJuliaML:masterfrom
albheim:master

Conversation

@albheim
Copy link
Contributor

@albheim albheim commented Feb 11, 2019

As discussed in issue #25

@JobJob
Copy link
Member

JobJob commented Feb 11, 2019

Thanks!

I suppose we should at least call close(env) in the tests, even if it doesn't do much, after this line seems like an ok spot:

println("microsecs/step (lower is better): ", t*1e6/steps)

I don't think we need to add calls to render because they might not work on the CI machines.

Speaking of CI @iblis17 do you have any idea what the failures are?

@iblislin
Copy link
Member

The CI failure is caused by recent Gym release.
It's already fixed on their master: openai/gym#1312

@iblislin
Copy link
Member

Maybe we should fix the gym version to avoid the uncertainty from upstream.
e.g.

pip install 'gym[atari]'==x.x.x

@JobJob
Copy link
Member

JobJob commented Feb 11, 2019

Maybe we should fix the gym version to avoid the uncertainty from upstream.

Yes 💯

Also, I think we should test using Python 3, instead of Python 2, if possible. Python 2 people are a dying breed.

@iblislin
Copy link
Member

Is PyCall python3-ready?

@JobJob
Copy link
Member

JobJob commented Feb 11, 2019

Is PyCall python3-ready?

yep, I think it's python3 by default if it installs python via conda too

@iblislin
Copy link
Member

ah, okay.

@iblislin iblislin closed this Feb 12, 2019
@iblislin iblislin reopened this Feb 12, 2019
@JobJob
Copy link
Member

JobJob commented Feb 13, 2019

I forced pushed to your master, sorry about that, should have just merged #28 rather than rebasing on top of it, but I got a little reckless not realising I had to push to your branch.

Anyway, to get back to normal on your machine you probably just want to (note this will blast away any changes you made so commit or stash them first)

git checkout master
git reset --hard 08b87cb4220f2dd5cbef558f345efe14bb5d2a81
git pull

Anyway, I added a call to close in the tests.

Thanks!

@JobJob JobJob merged commit 5216cfd into JuliaML:master Feb 13, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants