ALE 0.6

[JesseFarebro]

This PR updates the ALE to version 0.6 which includes game mode/difficulties as well as improved game support over 0.5. Once this is merged I have PR for Gym which enables selecting modes/difficulties.

Since this is a major change to the base atari-py package I also bumped the semver to 0.2.0. If you want to manage versioning yourself I can remove this commit. Let me know if you need any changes or edits to this PR.

CC @mcmachado @mgbellemare

[christopherhesse]

Looks great! Looks like gym doesn't select the version of atari py very well and may be broken by this upgrade. @pzhokhov what do you think?

[christopherhesse]

Also heads up that @pzhokhov is working on windows support so you might need to rebase on that: #50

[JesseFarebro]

@christopherhesse what would be blocking from Gym's side? There aren't any major changes in 0.6 except for the addition of modes/difficulties and improved game support. There were no API changes which would affect how Gym currently interacts with the ALE. I tested my PR locally with Gym and everything was working as expected.

Regarding #50 that sounds good to me, there seems to be minimal change to the ALE itself. I'll wait for #50 to be finalized and merged before proceeding?

[christopherhesse]

Good idea to wait for #50.

I was worried that ALE 0.6 was not backwards compatible (for instance it's 0.6 instead of 0.5.1). If this is backwards compatible (that is, old gym will work with atari py 0.1 and 0.2, then that's fine).

That said we should probably still use a more restrictive version selector in gym's setup.py in case we make backward-incompatible changes.

[JesseFarebro]

I'll double confirm with @mcmachado but from what I can see there weren't any breaking changes with the API between 0.5 and 0.6 except for the addition of game modes/difficulties.

I'm subscribed to #50 and hopefully, it will get merged soon 👍.

Once this PR gets merged I have a PR ready for Gym which allows for the selection of modes/difficulties. We can also work together to restrict the versioning of atari-py within the same PR for Gym.

[christopherhesse]

I don't mean just breaking changes with the API, if the games were changed in such a way as to warrant a new env id (like -v5) suffix, that would also count.

[JesseFarebro]

I'm not sure I understand what you mean by the games changing? Would you bump the version of a game if there were bug fixes, e.g., introducing episode termination upon reaching max score?

One thing I wanted to speak to you about was having an environment id for specific combinations of modes and difficulties in Gym. For example, in a recent paper, we abbreviate modes and difficulties with mXdY where X is the mode index and Y is the difficulty index. Would Gym be open to environment IDs like Freeway-m1d1-v0? This wouldn't alter the default Freeway-v0.

[christopherhesse]

Anything that would make the results not comparable across papers. A bug fix that only affects when you reach max score seems less likely to meet that criteria.

I suspect there are too many env ids already for atari envs in gym, we currently register 6 variants for each game: https://github.com/openai/gym/blob/7006c7a182a991b9409c206c56abc121eaffafb5/gym/envs/__init__.py

I think there it might be good to use the new support for kwargs to gym.make openai/gym#1301 and re-use the existing env ids

[mgbellemare]

PIggybacking on this thread to make a comment: there is also some discussion of fixing games where agents "roll" the score on the ALE repository: Farama-Foundation/Arcade-Learning-Environment#262 Would be good to have a longer discussion re: reproducibility to this effect.

[JesseFarebro]

@christopherhesse The Python 3.5 macOS build seems to be giving me some problems. I'm not sure why it can't find the smoke test. I would investigate further but it's kind of hard without being able to access debug mode on the build. Otherwise, all other builds are working as expected.

I will outline a couple of changes I made from the Windows PR.

1. We build the libraries in the current source directory. This just required removing the build directory when loading the libraries as well as linking to the new location in package_data.txt.

atari-py/atari_py/ale_python_interface.py

Lines 13 to 18 in f05ab1d

	if os.name == 'posix':
	ale_lib = cdll.LoadLibrary(os.path.join(os.path.dirname(__file__),
	'ale_interface/libale_c.so'))
	else:
	ale_lib = cdll.LoadLibrary(os.path.join(os.path.dirname(__file__),
	'ale_interface/ale_c.dll'))

2. Now in package_data.txt I'm not sure what this directory is referring to? I ended up removing it.

atari-py/atari_py/package_data.txt

Line 3 in d7f596d

ale_interface/build/ale

It's also worth noting that I haven't tested the Windows builds locally. I don't have access to a machine with Windows.

Once we can resolve the Python 3.5 build everything should be ready to merge.

[christopherhesse]

Great! Thanks for updating your PR! We can likely look more at this at the end of the week.

For your packaging changes, please make sure that if you do pip install -e in a newly checked out repo, everything works correctly. Also if you uninstall atari_py, then build using python setup.py bdist_wheel and install that wheel and make sure it works. The tests should cover the second one but I doubt they cover the first one (maybe they should though).

Also from the test log it looks like we install atari_py, then uninstall it and replace it with one of the pre-existing binaries (gym now depends on atari_py 0.1.*). It's likely that this is incorrect and we should install atari_py after gym[atari] to make sure we overwrite it with the local version. Or else not install gym[atari] at all, just install gym + deps.

[JesseFarebro]

Both installing with pip install -e . and installing the wheel from python setup.py bdist_wheel are working as expected. Just let me know if you need anything else from my end.

[christopherhesse]

I think you may want to look at the problem with the tests maybe running against the wrong version of atari_py since I don't think this is a problem on master.

[JesseFarebro]

I looked into it more and it turns out this was an issue with how Gym would resolve the atari-py package. I removed the version bump to 0.2 and the package builds successfully. As you mentioned previously, there should probably be some changes to how Gym resolves atari-py.

[christopherhesse]

gym could be changed to use the new version of atari-py, but it seems odd to me that atari-py cares what version of atari-py gym depends on.

@pzhokhov should we just remove the gym lines from the atari-py tests here? https://github.com/openai/atari-py/blob/master/config.sh#L9 It seems like those tests should be tests that gym runs.

[JesseFarebro]

It also seems weird you run pytest on the installed package. Shouldn't you just run it locally? If gym resolves a different version of atari-py it won't even run test cases against the PR, it would run them against the resolved package.

[christopherhesse]

Yeah, that's what I was getting at in #49 (comment)

I believe pytest is run against the installed package because we want to make sure that the wheel works before we deploy that. Specifically we are not just testing that python setup.py develop then pytest works, but that the wheel includes any shared libraries and can import them after installation.

[christopherhesse]

Sorry we didn't get to this this week, hopefully we can look at it next friday though.

[christopherhesse]

In the meantime, it would be useful to fix the test setup if you have time.

[JesseFarebro]

Sure, do you want this in a different PR? Seems unrelated to the ALE.

[christopherhesse]

It's unrelated to ALE, but is necessary for this PR, a separate PR would be fine.

[JesseFarebro]

You are talking about removing gym from the test case outlined above? Replacing that with a proper test to verify whether packaging of the wheel succeeded?

I already fixed all the other build issues with this PR already (i.e., all your tests pass for this PR). I didn’t know there’s a test considered necessary for this PR to be merged? The gym fix would be nice to have and I’m happy to do so if that’s what your referring to.

[christopherhesse]

Yeah, that test case. I'm confused though, I thought that was installing the wrong version of atari_py and running the tests against it, but I don't see that behavior. Did this PR change the version number previously?

[JesseFarebro]

Yes, I previously had a commit that changed the version number and this was causing the issue you mention. I removed said commit as it seems like your CI infrastructure was handling versioning.

I think it’s still advisable to bump the version to 0.2.X now that we use ALE 0.6. I just deferred the issue of versioning until after this is merged.

[christopherhesse]

Okay, we can address it in a later PR then, it won't block this PR just the next release.

[christopherhesse]

Thanks for the PRs @JesseFarebro!