Conversation
Codecov ReportBase: 91.32% // Head: 91.20% // Decreases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## main #1508 +/- ##
==========================================
- Coverage 91.32% 91.20% -0.12%
==========================================
Files 15 15
Lines 1303 1308 +5
Branches 223 225 +2
==========================================
+ Hits 1190 1193 +3
- Misses 80 81 +1
- Partials 33 34 +1
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
|
Need to resort to a somewhat in between implementation (I think that actually most of the times just the constant time condition is evaluated): any time the agent.pos position is not preassigned to where you want to place the agent in the grid it takes a constant amount of time, otherwise it goes to scan the list |
mesa/space.py
Outdated
| """Place the agent at the specified location, and set its pos variable.""" | ||
| x, y = pos | ||
| if agent not in self.grid[x][y]: | ||
| if agent.pos != pos or agent not in self.grid[x][y]: |
There was a problem hiding this comment.
This needs to be agent.pos is None. The case when agent.pos is something that is not None, but is not pos either, means something is wrong. The agent might exist in both self.grid[x][y] and somewhere else.
There was a problem hiding this comment.
Good catch! I was thinking something similar, where agent.pos is simply set to something different, but dismissed it as unlikely. But agents being on grid and another space (e.g. network) is something I see happening. And in that case the agent.pos might as well be set to None by the network.
I think the current implementation is the most defensive mechanism, not a bad thing.
There was a problem hiding this comment.
very good points but what you are saying @Corvince make me think about something to resolve ambiguities, in a sense as you point out it doesn't make much sense to have a pos variable shared by all spaces, we should create a pos variable for each agent related to each space to which the agent belongs!
There was a problem hiding this comment.
By somewhere else, I meant self.grid[x_other][y_other]. agent.pos is not None happens when the modeler calls consecutive place_agent's without ensuring that an agent is removed from the grid before a place_agent happens.
There was a problem hiding this comment.
yes, indeed this means that the current implementation is prone to errors
|
I'm curious, with Python, in a conditional if agent.pos is None or agent not in self.grid[x][y]:If the first condition proves false, does Python still evaluate the second condition? Or does it continue directly without evaluating it? |
|
@EwoutH i think you meant "if the first condition proves true" yes it doesn't evaluate the second one in this case |
|
I meant that indeed. That also means that the order of the or statement matters. That's interesting, thanks! |
|
LGTM |
5 participants
The condition tested now is a constant time operation while before it was a linear time operation in relation to the size of the list of agents