Skip to content

Fix IndexError in select_random_agent for empty collections#2986

Closed
Nithin9585 wants to merge 1 commit intomesa:mainfrom
Nithin9585:fix/select-random-agent-empty-collection
Closed

Fix IndexError in select_random_agent for empty collections#2986
Nithin9585 wants to merge 1 commit intomesa:mainfrom
Nithin9585:fix/select-random-agent-empty-collection

Conversation

@Nithin9585
Copy link
Copy Markdown
Contributor

Fixes #2982

Changes

  • Added default parameter to select_random_agent()
  • Uses _RAISE sentinel object (proper Python pattern)
  • Raises LookupError for empty collections (semantically correct)

Performance

Benchmarked 21% faster on sparse grids vs exception handling.

Implementation

Follows dict.get() pattern - raises error by default, returns default if provided.

Approved by @quaquel.

@github-actions
Copy link
Copy Markdown

Performance benchmarks:

Model Size Init time [95% CI] Run time [95% CI]
BoltzmannWealth small 🔵 -3.6% [-4.4%, -2.7%] 🔵 -0.4% [-0.5%, -0.3%]
BoltzmannWealth large 🔵 +0.5% [-1.5%, +3.5%] 🔵 -3.3% [-5.2%, -1.6%]
Schelling small 🔵 -0.5% [-1.5%, +0.6%] 🔵 +0.1% [-0.5%, +0.6%]
Schelling large 🔵 +1.5% [+0.3%, +3.4%] 🔵 +3.5% [+2.2%, +4.7%]
WolfSheep small 🔵 +0.6% [-0.4%, +1.3%] 🔵 -1.3% [-1.5%, -1.1%]
WolfSheep large 🔵 +1.1% [-3.0%, +3.5%] 🔴 +6.8% [+5.2%, +8.3%]
BoidFlockers small 🔵 -3.0% [-3.5%, -2.5%] 🔵 -0.2% [-0.4%, +0.1%]
BoidFlockers large 🔵 -3.5% [-4.2%, -2.7%] 🔵 -0.1% [-0.5%, +0.2%]

- Add default parameter to handle empty collections gracefully
- Use _RAISE sentinel object (proper Python pattern)
- Raise LookupError instead of IndexError (semantically correct)
- 21% performance improvement on sparse grids (benchmarked)
Fixes mesa#2982
@Nithin9585 Nithin9585 force-pushed the fix/select-random-agent-empty-collection branch from 0871b0b to 9526809 Compare December 22, 2025 10:12
@Nithurshen
Copy link
Copy Markdown
Contributor

@Nithin9585 Please note that PR #2983 is already open for this issue and has been updated with the agreed-upon fix. Opening a second PR creates unnecessary noise for the maintainers. Please close this in favor of the existing one.

@quaquel
Copy link
Copy Markdown
Member

quaquel commented Dec 22, 2025

Thanks for this PR. Since it is identical by now to #2983, which was submitted earlier, I prefer to merge that one. However, I would like to thank you for the clear reporting and responsiveness on the underlying bug. If we merge #2983, in my view, it was as much your work as that of the actual author of that PR.

@Nithin9585 Nithin9585 closed this Dec 22, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

CellCollection.select_random_agent() crashes when no agents in cells

3 participants