Skip to content

[Bug]: agentsets_by_type won't work with duplicate types #124

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

Open
Ben-geo opened this issue Mar 18, 2025 · 8 comments
Open

[Bug]: agentsets_by_type won't work with duplicate types #124

Ben-geo opened this issue Mar 18, 2025 · 8 comments
Assignees
Labels
bug Something isn't working
Milestone

Comments

@Ben-geo
Copy link
Collaborator

Ben-geo commented Mar 18, 2025

📝 Description

the function agentsets_by_type(self) in the file agents.py
won't work with duplicate types
it will only store the latest agents_df

🔄 Steps to Reproduce

No response

🎯 Expected Behavior

Ideas :

  • append into the existing to keep each fixture separate
  • append all together
    i think the first method is better

🚨 Actual Behavior

No response

🖥️ Python Version

No response

Operating System

No response

Relevant Packages

No response

📊 Relevant Log Output

No response

➕ Additional Context

No response

@Ben-geo Ben-geo added bug Something isn't working triage Needs initial review and categorization labels Mar 18, 2025
@Ben-geo
Copy link
Collaborator Author

Ben-geo commented Mar 18, 2025

@adamamer20 Look into this and let me know if this sounds good or if I should give more context
also can you assign this to me

@adamamer20
Copy link
Member

Do you mean adding a specific AgentSetDF subclass (eg. ExampleAgentSetPandas) twice into a single model?
If this is the case, I don't think we should allow to have more than one instance of the same type in a single model (and thus in AgentsDF) since the best practice is to keep all agents of the same type in a single DF anyway. If this is not clear we can make an addition to the documentation and handle the case when the user adds the same class to the model.
Can you give me some code to reproduce the bug?

@Ben-geo
Copy link
Collaborator Author

Ben-geo commented Mar 19, 2025

In my understanding, AgentsDF is a class that stores all agents of the model. Suppose we have two instances of ExampleAgentSetPolars, each representing a different agent type (similar to the Prey and Predator example in the ModelDF documentation). Since these agents exhibit different behaviors, they should belong to separate AgentSetDF instances.

However, when retrieving agent sets from AgentsDF using AgentsDF.agentsets_by_type, the first agent set (e.g., Prey) gets overwritten by the second one (e.g., Predator).

Does this interpretation align with how AgentsDF is expected to function, or am I misunderstanding something?

@adamamer20
Copy link
Member

adamamer20 commented Mar 20, 2025

If they are different agent "types" (eg. Prey and Predator), they should be instances of different classes (in this case Prey and Predator), not of the same class.

If the bug happens anyway with different classes, we should handle it.
Otherwise, we might need to check if the user is trying to add different instances of the same class (eg. multiple Prey classes) to the model and raise an error or a warning.

@Ben-geo
Copy link
Collaborator Author

Ben-geo commented Mar 20, 2025

They will be different AgentSetDF but within the same AgentsDF so when we try to group by types within AgentsDF they should give multiple AgentSetDF for the key ExampleAgentSetPolars

like say a prey and Predator both are ExampleAgentSetPolars within AgentsDF
if we run AgentsDF.agentsets_by_type we will only get one of them (which ever was iterated over last) as key will be common for the dictionary and we are assigning both to the same key
so it will write over the first occurrence

I hope this makes sense

@adamamer20
Copy link
Member

Mmm, so there is a problem with classes having the same base class? But we have a test for that and it seems to work.
Can you give me a reproducible example to understand better, please?

@Ben-geo
Copy link
Collaborator Author

Ben-geo commented Mar 21, 2025

see the reason the tests work cause they dont take care of all the edge cases
consider this

    @pytest.fixture
    def fix_AgentsDF( fix1_AgentSetPandas: ExampleAgentSetPandas, fix2_AgentSetPolars: ExampleAgentSetPolars,) -> AgentsDF:
        model = ModelDF()
        agents = AgentsDF(model)
        agents.add([fix1_AgentSetPandas, fix2_AgentSetPolars])
        return agents

    def test_agentsets_by_type(self, fix_AgentsDF: AgentsDF):
        agents = fix_AgentsDF
        result = agents.agentsets_by_type
        assert isinstance(result, dict)
        assert isinstance(result[ExampleAgentSetPandas], AgentsDF)
        assert isinstance(result[ExampleAgentSetPolars], AgentsDF)
        assert result[ExampleAgentSetPandas]._agentsets == [agents._agentsets[0]]
        assert result[ExampleAgentSetPolars]._agentsets == [agents._agentsets[1]]

lets change the fix

    def fix_AgentsDF(
        fix1_AgentSetPolars: ExampleAgentSetPolars,
        fix2_AgentSetPolars: ExampleAgentSetPolars,
    ) -> AgentsDF:
        model = ModelDF()
        agents = AgentsDF(model)
        agents.add([fix1_AgentSetPolars, fix2_AgentSetPolars])
        return agents

    def test_agentsets_by_type(self, fix_AgentsDF: AgentsDF):
        agents = fix_AgentsDF
        result = agents.agentsets_by_type
        assert isinstance(result, dict)
        assert isinstance(result[ExampleAgentSetPolars], AgentsDF)
        assert result[ExampleAgentSetPolars]._agentsets == [agents._agentsets[1]] # this will pass
         assert result[ExampleAgentSetPolars]._agentsets == [agents._agentsets[0]] # this will raise an error obviously coz 2 agent sets are of the type ExampleAgentSetPolars

so even though we try to get agetnsets_by_type we will only get the last agent set that was added to AgentsDF

I really hope this explains what I am trying to say TT
unless its an error in how I am processing AgentsDF behavior
lemme know if my thought process is wrong

@adamamer20
Copy link
Member

Alright perfect, so the initial idea was to allow a single AgentSetDF type since, conceptually, all agents of the same type should be grouped in one class. In other words, Prey would have its own AgentSetDF class and Predator would have its own. However, we might want to allow users the flexibility to decide what they prefer. Feel free to open a PR that returns a list of AgentSet.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants