search

Farama-Foundation / Arcade-Learning-Environment

The Arcade Learning Environment (ALE) -- a platform for AI research.
https://ale.farama.org/
GNU General Public License v2.0
2.14k stars 420 forks source link

Some not strictly supported ROMs cause assertion failure in ColourPalette::getRGB because they mix greyscale and colour. #195

Closed nczempin closed 6 years ago

nczempin commented 7 years ago

With air_raid.bin (and a few others that I didn't write down), I get this when running the python example agent with SDL: .../Arcade-Learning-Environment/src/common/ColourPalette.cpp:51: void ColourPalette::getRGB(int, int&, int&, int&) const: Assertion (val & 0x01) == 0' failed.

nczempin commented 7 years ago 
# bad: [a525e572777a12d06701cc5327d2c6b81d4cf0fd] Add Kaboom! rom
git bisect bad a525e572777a12d06701cc5327d2c6b81d4cf0fd
# good: [cf248f59c56cf7314c1d91112b56745f43bab80a] Merge pull request #108 from mgbellemare/dev
git bisect good cf248f59c56cf7314c1d91112b56745f43bab80a
# good: [5a6a53cf2ef928c42cdf5b62bca9c33d38b05de7] Merge branch 'master' of git://github.com/mcmachado/Arcade-Learning-Environment
git bisect good 5a6a53cf2ef928c42cdf5b62bca9c33d38b05de7
# good: [2b5c3796a8f1536ffe8219db0e25ea190d3727aa] Merge pull request #171 from ppwwyyxx/master
git bisect good 2b5c3796a8f1536ffe8219db0e25ea190d3727aa
# bad: [c63932eb10cb25623d09ccf28e869e6b4029d5d3] Merge pull request #182 from mgbellemare/fix-color-averaging
git bisect bad c63932eb10cb25623d09ccf28e869e6b4029d5d3

So Merge pull request #182 from mgbellemare/fix-color-averaging git bisect bad c63932eb10cb25623d09ccf28e869e6b4029d5d3 introduced this issue

mgbellemare commented 7 years ago

@nczempin , that's when the assert was introduced. Can you confirm that this is also an issue with Carnival?

nczempin commented 7 years ago

yes, same assertion failure on carnival.

nczempin commented 7 years ago

hm, it seems that now all the roms I'm trying are failing this way. I'm pretty sure I had things working on the master HEAD before

mgbellemare commented 7 years ago

Is this with the default setting, or some additional configuration? What is the value 'val' that's triggering the assert?

On Sat, Apr 1, 2017 at 5:30 PM, Nicolai Czempin notifications@github.com wrote:

yes, same assertion failure on carnival.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/mgbellemare/Arcade-Learning-Environment/issues/195#issuecomment-290930861, or mute the thread https://github.com/notifications/unsubscribe-auth/ACPczmGjkRVBwFlbbfSGXDz6xsE_rANpks5rrnuVgaJpZM4Mwfei .

nczempin commented 7 years ago

I'm just running the python example with USE_SDL set to true

and it only occurs in this setting, when it's false everything is fine.

nczempin commented 7 years ago

What is the value 'val' that's triggering the assert?

129 with 3/3 roms I've tried

nczempin commented 7 years ago

I take that back; I seem to be using the same ROM every time, air raid, no matter what actual rom I pass. I will fix that and report more details.

nczempin commented 7 years ago

Okay, I ran it with all the roms that are included in atari_py (which is used by gym), which is slightly less than what is supported by the current ALE, the assertion failure occurs only on air_raid.bin 35be55426c1fec32dfb503b4f0651572 and amidar.bin 056f5d886a4e7e6fdd83650554997d0d

nczempin commented 7 years ago

The offending values are 129 for air_raid and 1 for amidar.

nczempin commented 7 years ago

So I assume you can't reproduce these errors, @mgbellemare ? I'm on Ubuntu 16.04.04 LTS, and I can reproduce them reliably.

I guess I can look into it a little more, but you'd of course know the colour-averaging code better than I.

nczempin commented 7 years ago

It is within SDL-specific code: void DisplayScreen::display_screen()

nczempin commented 7 years ago

So a quick workaround would be to put an #ifndef USE_SDL around the assertion. Not sure how this would affect the functionality

nczempin commented 7 years ago

When I do that, this is what I get:

frame 0000 image

frame 0001 image

It seems to randomly switch between grayscale and colour.

nczempin commented 7 years ago

And with amidar, the first two frames are just black screens, frame 00003 is the game in grayscale, and from then on all the frames are coloured.

nczempin commented 7 years ago

Playing the rom in Stella, it does intermittently go to greyscale, I'm not sure exactly when.

BTW I'm not sure if this is relevant, but when you run it via ALE, instead of being grayscale, it has a sort of bluish tint when it would normally go to greyscale.

mgbellemare commented 7 years ago

That's interesting. The assert is exactly there to guarantee that you're either playing in colour or grayscale mode. if there are some games that automatically switch between the two, we'll have to revisit this design choice.

Re: air_raid, the game doesn't seem to work very well on Stella. It's not part of the 60 games typically used, so I would just set it aside.

Re: amidar. The easiest solution is probably to skip the first 3 frames. Can you confirm that this doesn't happen past the third frame? Also, that bluish tint -- do you have code that I can use to reproduce this behaviour?

On Sun, Apr 2, 2017 at 12:14 PM, Nicolai Czempin notifications@github.com wrote:

Playing the rom in Stella, it does intermittently go to greyscale, I'm not sure exactly when.

BTW I'm not sure if this is relevant, but when you run it via ALE, instead of being grayscale, it has a sort of bluish tint when it would normally go to greyscale.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/mgbellemare/Arcade-Learning-Environment/issues/195#issuecomment-290979790, or mute the thread https://github.com/notifications/unsubscribe-auth/ACPczt46aEY4Q_TjJJpT7Bv326UZj7QYks5rr4MUgaJpZM4Mwfei .

nczempin commented 7 years ago

Re: amidar. The easiest solution is probably to skip the first 3 frames. Can you confirm that this doesn't happen past the third frame?

Only empirically, by looking at the PNGs generated by the ale-videoRecordingExample in a folder: image

I visually scanned all the way to 002310.png

mgbellemare commented 7 years ago

Are you in SECAM mode somehow? My frame 000002 is yellow, with no grayscale (or asserts) in-between: 000002

nczempin commented 7 years ago

that bluish tint -- do you have code that I can use to reproduce this behaviour?

I pretty much just use docs/examples/python_example.py or ale-videorecordingExample

I compiled with cmake -DUSE_SDL=ON -DUSE_RLGLUE=OFF -DBUILD_EXAMPLES=OFF

nczempin commented 7 years ago

Re: air_raid, the game doesn't seem to work very well on Stella. It's not part of the 60 games typically used, so I would just set it aside.

For me it is more playable in ALE than in Stella, where I get intermittent black screens.

nczempin commented 7 years ago

Are you in SECAM mode somehow? My frame 000002 is yellow, with no grayscale (or asserts) in-between:

A.L.E: Arcade Learning Environment (version 0.5.1) [Powered by Stella] Use -help for help screen. Game console created: ROM file: air_raid.bin Cart Name: Air Raid (Men-A-Vision) Cart MD5: 35be55426c1fec32dfb503b4f0651572 Display Format: AUTO-DETECT ==> PAL ROM Size: 4096 Bankswitch Type: AUTO-DETECT ==> 4K

A.L.E: Arcade Learning Environment (version 0.5.1) [Powered by Stella] Use -help for help screen. Game console created: ROM file: amidar.bin Cart Name: Amidar (1983) (Parker Bros) (PAL) [!] Cart MD5: 056f5d886a4e7e6fdd83650554997d0d Display Format: AUTO-DETECT ==> PAL ROM Size: 4096 Bankswitch Type: AUTO-DETECT ==> 4K

The Amidar in SDL ALE is reddish, as in the pngs, but the air raid looks different on-screen and in the recorded video:

image

mgbellemare commented 7 years ago

Ah, the standard amidar is the NTSC one, not PAL. Seems like we do need those MD5s checks!

On Sun, Apr 2, 2017 at 12:41 PM, Nicolai Czempin notifications@github.com wrote:

Are you in SECAM mode somehow? My frame 000002 is yellow, with no grayscale (or asserts) in-between:

A.L.E: Arcade Learning Environment (version 0.5.1) [Powered by Stella] Use -help for help screen. Game console created: ROM file: air_raid.bin Cart Name: Air Raid (Men-A-Vision) Cart MD5: 35be55426c1fec32dfb503b4f0651572 Display Format: AUTO-DETECT ==> PAL ROM Size: 4096 Bankswitch Type: AUTO-DETECT ==> 4K

A.L.E: Arcade Learning Environment (version 0.5.1) [Powered by Stella] Use -help for help screen. Game console created: ROM file: amidar.bin Cart Name: Amidar (1983) (Parker Bros) (PAL) [!] Cart MD5: 056f5d886a4e7e6fdd83650554997d0d Display Format: AUTO-DETECT ==> PAL ROM Size: 4096 Bankswitch Type: AUTO-DETECT ==> 4K

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/mgbellemare/Arcade-Learning-Environment/issues/195#issuecomment-290981102, or mute the thread https://github.com/notifications/unsubscribe-auth/ACPczpgUa2qBZWBpWi9zieK-97fnLW_Lks5rr4mBgaJpZM4Mwfei .

nczempin commented 7 years ago

I think I got those roms either from openai/gym or from the zip file that someone posted in the Google Group. I have tons of others; which md5s are you using for amidar and air raid?

nczempin commented 7 years ago

Okay, so it works for the NTSC version of Amidar (1983) (Parker Bros) acb7750b4d0c4bd34969802a7deb2990

nczempin commented 7 years ago

same for air raid: b68cffd84868a11826179b99fbc64489 works as expected

mgbellemare commented 7 years ago

The SDL visualization does indeed look different:

blue-tint

All colours are slightly off.

It's only the ALE visualizer, though. Recorded frames are fine. I don't have time to look into this but do let me know if you find anything.

nczempin commented 7 years ago

I don't have time to look into this but do let me know if you find anything.

It's not high on my priority list either. We have the workaround of using the correct ROMs; it would be great if you could run md5sum on yours.

mgbellemare commented 7 years ago

MD5 (air_raid.bin) = 35be55426c1fec32dfb503b4f0651572 MD5 (amidar.bin) = acb7750b4d0c4bd34969802a7deb2990

nczempin commented 7 years ago

Maybe you could add your whole set to the issue #123, or just those that deviate from https://github.com/mgbellemare/Arcade-Learning-Environment/issues/123#issuecomment-289593953.

I guess a text file like originally suggested would be a good start, but just to have the data somewhere would help some people.

Eventually something like "Warning: you're using an unsupported version of air_raid.bin", perhaps even an error message and exit (overridable by parameter?) might be useful.

Overall, right now my personal preference for helping out in this project would be to significantly increase the number of possible "targets", be it more actual ROMs or more support for additional modes in already supported ROMs.

mgbellemare commented 7 years ago

Having a warning sounds like a good idea. I've pushed the md5 list now, maybe you can have a look and see if printing a warning is easy?

nczempin commented 7 years ago

It will be very easy; the information is already there. I'll implement this next.

I had an annoying segmentation fault when the ROM name resolution fails; its fix would be one small part of my "make it easier to use and add ROMs" branch that I'd propose to bundle with the md5 check.

On 9 Apr 2017 10:51 pm, "Marc G. Bellemare" notifications@github.com wrote:

Having a warning sounds like a good idea. I've pushed the md5 list now, maybe you can have a look and see if printing a warning is easy?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/mgbellemare/Arcade-Learning-Environment/issues/195#issuecomment-292812076, or mute the thread https://github.com/notifications/unsubscribe-auth/ABsuncncIcj-rSx90TF4YS52UE1HGF9Qks5ruUTUgaJpZM4Mwfei .

mgbellemare commented 6 years ago

Closing -- the issue seems to be with air raid, which isn't quite supported.