Observation does not distinguish between empty cells and unobserved cells #35
Comments
|
You're not the first to bring this up, which suggests it's probably a good idea to add this information to observations. Your analysis of the options is pretty spot on, and it's good that you take code breakage into account, most people pay no attention to this. I think I might lean towards option 2. The unseen cell type could take integer value 0, and everything else would be shifted up by 1. It probably won't break code. Actually, the upside is that a neural network (or other algorithm) trained with this new observation format should be able to take advantage of the extra information without any changes to the code. |
|
Good point, I agree option 2 is both the least invasive and requires less changes to models to exploit the new info: "unseen" (or "n/a") can just be a separate value in the observation image. I assume this new value would be used in all three channels, i.e. the other channels would also be shifted by one? Related: The third channel is currently only used to indicate whether a door is open or closed, but I tend to think of it as a more general "cell/object state". Do you share this view? If new objects with a modifiable state were to be introduced, would their state also inhabit the third channel? |
|
This might require a new Object to be defined; The least invasive way seems to add an Unseen / Unknown cell type, but this seems backwards to me: If anything, an Empty cell is more of an actual physical property of the grid, compared to an Unseen. This may require more work to carefully make sure nothing breaks, but what do you think about either:
I prefer number 1, but I obviously lack a lot of understanding in how this may influence and break everything else, so it's your choice. |
|
In terms of the least code breakage, it would be best not to add new cell types. I would encode Unseen as (0, 0, 0) on all 3 channels and increment the existing cell type integer values by 1. For |
|
Just to be clear, you're suggesting that the encodings for the second and third channels should not change, right? i.e. In the color channel, 0 should remain red, which at the same time means that both empty cells and unseen cells will be encoded as having the same code as red? |
|
Yes |
…ls (Farama-Foundation#35) * add ('unseen', 0) to `OBJECT_TO_IDX` dictionary; all other values are shifted up by 1 to make space for the new value. * update `Grid.encode` to take an optional `vis_mask` argument, and return an image which now distinguishes between unseen and empty cells (values respectively 0 and 1). * update `Grid.decode` to handle the above changes; values 0 and 1 in input array are both mapped to None in output grid. * make Grid.decode() a static method; it was already used as a static method but was missing the appropriate decorator. * update "observation encode/decode roundtrip" test in run_tests.py; the visualization mask necessary to perform the test is found by manually checking the first channel of the image for value 0.
…ls (#35) (#36) * add ('unseen', 0) to `OBJECT_TO_IDX` dictionary; all other values are shifted up by 1 to make space for the new value. * update `Grid.encode` to take an optional `vis_mask` argument, and return an image which now distinguishes between unseen and empty cells (values respectively 0 and 1). * update `Grid.decode` to handle the above changes; values 0 and 1 in input array are both mapped to None in output grid. * make Grid.decode() a static method; it was already used as a static method but was missing the appropriate decorator. * update "observation encode/decode roundtrip" test in run_tests.py; the visualization mask necessary to perform the test is found by manually checking the first channel of the image for value 0.
I think it would be beneficial (at least to me) to include the option of distinguishing between a cell which is observed to be empty, and a cell which is not seen at all (e.g. behind a wall). This is currently not the case, as both scenarios correspond to 0 values in the observation image.
I would probably try to do this in my own fork, but if there is any interest in pulling this to the main repo I would like to discuss it here because there's multiple ways to approach this:
Adding an item to the observation dictionary.
pros: does not change observation image, i.e. less likely to break existing code. Completely separates "what is seen" from "whether it is seen", so that the user can choose whether to use just one of both.
cons: The original ImgObsWrapper would ignore the addition and just return the original image.
Adding a cell type value to the observation image; perhaps this can be done by adding a new cell type altogether "Unseen" which will only be used in observation grids?
pros: minor change to observation image, less likely to break code
cons: Arguably awkward to add a type which is not associated with the state of the true environments and its objects.
Adding a binary channel to the observation image.
pros: dedicated channel, "unseen" is not a physical property inherent to the environment so it makes sense to separate it from things like the actual items.
cons: major change to observation image, more likely to break existing code.
Personally, I think option 1 would work best: Existing code will continue working as before, and methods which want to exploit the new mask are able to do so. New observation wrappers can be added to handle the case where the user wants just the image, or the image and the "mask".
Thoughts?
The text was updated successfully, but these errors were encountered: