- 
          
- 
                Notifications
    You must be signed in to change notification settings 
- Fork 1.1k
Fix PropertyLayer Visualization on HexGrids #2864
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
Conversation
…on. Added pngs showing differences between Hex and OVN grids.
…operation before ravel operation.
…on. Added pngs showing differences between Hex and OVN grids.
This reverts commit a77222f.
…operation before ravel operation.
This reverts commit a77222f.
for more information, see https://pre-commit.ci
add more maps to example
for more information, see https://pre-commit.ci
| Performance benchmarks: 
 | 
| Performance benchmarks: 
 | 
| Thanks for the detailed explanation and for fixing the issue. You’re right — the data was mapped incorrectly, and transposing it does indeed fix the problem, so this PR looks good to me. I also tried running your example but couldn’t get it to work in Altair — not sure why. You can go ahead and remove the example and the related files from this PR now. | 
| Thanks for the detailed bug report (#2859) and PR @flucco! @Sahil-Chhoker thanks for picking up the review. @quaquel you might also find this interesting and want to skim this through. | 
| I agree with @Sahil-Chhoker, the transpose is the right fix. However, all the other changes seem not to be relevant so once those are removed this can be merged. | 
| 
 Thank you! I will remove my example folder from the PR. It's still in my bug_example branch for anyone's future reference. RE Altair, the example I made only works with Solara on the Matplotlib backend. I only ever used the Matplotlib code in my own project, so that was the only bug I found. After looking at the Altair version myself, I think there is a separate issue with the Altair visualizations in the latest version of the code. I haven't dug into it much but if you run the unmodified Sugarscape example with the current code, you'll see a bug that looks very similar to what I described and fixed here. So either there's a similar problem in the Altair code or the example itself needs modification to work with the latest updates. | 
| @flucco feel free to open a new PR if that's easier! Whatever works for you | 
| 
 I would not be surprised if an identical issue exists in altair. This transpose stuff has tricked me up several times before with property layer visualization. | 
| I also dug around a bit, the property layer data mapping for  | 
Summary
Fix a visualization error for PropertyLayers on HexGrids where color values were displayed in the wrong places. Add a transpose operation to put the color values in the right order.
Bug / Issue
Issue 2859. I found that when trying to display PropertyLayers on Hexagonal Grids, color values are consistently shown in the wrong places on the grid. The visualizations for hexagonal grids look dramatically different from the visualizations for orthogonal grids. You would expect the visualizations to look similar besides the small differences caused by the hexagonal cell layout.
Implementation
The issue was in the draw_propertylayer function in matplotlib_backend.py. The code for the HexGrid visualization flattened the W x H PropertyLayer data into a 1D array, then matched those values to a collection of hexagonal grid cells arranged in H x W order. The different orders led to incorrect placement of values on the grid.
The one-line fix is to add a transpose operation (.T) before the ravel operation that flattens the data. This creates a 1D array of color values in the correct H x W order. I picked this approach because it's simple and matches the implementation used for orthogonal grids. Now, all cases use a simple transpose operation to adjust the data for graphing purposes.
Testing
I tested this fix in a modified version of the Sugarscape example, described in the issue and available in [this branch] (https://github.com/flucco/mesa/tree/bug_example) of my fork. I used the original sugarscape map, 50x50 modifications of that map which added asymmetrical elements, and modified maps that had varied W x H ratios.
Before the fix, the values aren’t shown in the right places on the grid. Agents should be in white squares due to consumption of resources, but the white squares appear in other locations. Here is the comparison at timestep 1 of Orthogonal and Hexagonal behaviors using the 60x40 grid. (Note: I changed the size of the agents to 7 and alpha to 0.8 as it's easier to see the color values). See also the examples shown in the issue.
Orthogonal:
Hexagonal:
Note the incorrect placement of the color values, as shown by the differences from the orthogonal grid and how the agents are not in white squares. Instead, the white squares that should correspond to the agents appear elsewhere in the grid.
After the fix, the behavior is correct in all cases I tested, and closely resembles the visualization of the original orthogonal model.
Hexagonal after fix:
I tested the fixed code on all the basic tests given and ran the formatting checks just to make sure.
Additional Notes
I admittedly didn’t do much additional testing because the change is extremely minimal, adding only a single operation in a single line of code. I believe it should only affect the precise thing I was looking to fix. Please let me know if there's anything else it would make sense to test.
I deleted my pull request and redid it because I didn't have the updated examples in my main branch. I wasn't sure if I should keep them in there but they're very easy to take out. All the examples are in the sugarscape_hex folder. The only actual change is to matplotlib_backend.py. (Also, I initially changed mpl_space_drawing.py, but that code is correct and shouldn't have been modified, so I changed it back).