-
Notifications
You must be signed in to change notification settings - Fork 12
Adds move_to_optimal function in DiscreteSpaceDF class #118
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
base: main
Are you sure you want to change the base?
Adds move_to_optimal function in DiscreteSpaceDF class #118
Conversation
Added the move_to function to allow agent movement based on specified attributes and ranking orders. The function considers neighborhood radius, sorting preferences, and optional shuffling for random movement. It ensures conflict resolution using priority-based selection, optimizing movement allocation. This enhances the agent-based model's flexibility and realism.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #118 +/- ##
=======================================
Coverage ? 89.24%
=======================================
Files ? 14
Lines ? 2344
Branches ? 0
=======================================
Hits ? 2092
Misses ? 252
Partials ? 0 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
This reverts commit df26a23.
Hi, @adamamer20 I have added the move_to method in the DiscreteSpaceDF class, It is more generalized version of get_best_moves. I hope it works well |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The implementation seems good!
There are 3 main points I'd like you to address before we merge:
- Add proper docstrings (NumPy style)
- Include unit tests in
tests\polars\test_grid_polars.py
- Consider adding an interface for this functionality to the
AgentContainer
classes (AgentSetDF
andAgentsDF
)
I'll benchmark soon to check if performance remains solid with the numba UDF approach, especially using lazyframes. If performance scales badly, we might need to iterate over columns directly or explore another solution.
mesa_frames/abstract/space.py
Outdated
raise ValueError("attr_names and rank_order must have the same length") | ||
if radius is None: | ||
if "vision" in agents.columns: | ||
radius = agents["vision"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a good idea but make sure to mention it in the docstring as it's a strong assumption.
2ebec10
to
df26a23
Compare
1.move_to function is renamed as move_to_optimize 2. Docstrings added 3. unit tests added to tests/polars/test_grid_polars 4. Interface for move_to_optimize added to AgentContainer, AgentsDF, AgentSetDF
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
…c/mesa-frames into optimizefunction
for more information, see https://pre-commit.ci
Hi @adamamer20, |
Unit tests for move_to_optimal method added updated move_to_optimal method and the docstring
This reverts commit 3d28340.
|
@adamamer20 I have fixed the test |
mesa_frames/abstract/agents.py
Outdated
obj = self._get_obj(inplace) | ||
|
||
# Apply move_to_optimal to each agent set in the container | ||
for agent_set in obj.agent_sets.values(): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
obj
does not have the agent_sets
attribute. Here in the abstract
file we define only the interface.
@adamamer20 is it fine now ? |
for agent_set in obj: | ||
agent_set.move_to_optimal( | ||
attr_names=attr_names, | ||
rank_order=rank_order, | ||
radius=radius, | ||
include_center=include_center, | ||
shuffle=shuffle, | ||
inplace=True, | ||
) | ||
|
||
return obj |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
again, no actual implementation should go here becuase it's an abstract interface. remove it completely. For the AgentSet you should do the implementation for AgentSetPolars.
@@ -616,6 +616,19 @@ def __str__(self) -> str: | |||
""" | |||
... | |||
|
|||
@abstractmethod | |||
def move_to_optimal( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Create a complete docstring like you did for GridPolars
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Added the move_to function to allow agent movement based on specified attributes and ranking orders. The function considers neighborhood radius, sorting preferences, and optional shuffling for random movement. It ensures conflict resolution using priority-based selection, optimizing movement allocation.