-
Notifications
You must be signed in to change notification settings - Fork 862
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
Feature Request: Agent DataCollection Can't Handle Different Attributes in ActivationByType #1419
Comments
@tpike3 I am unable to recreate the bug -- it looks like sugarscape_cg uses |
If you use this colab you will get the error https://github.com/edgeofchaos42/ComplexityExplorer/blob/main/Session_18_Data_Collector_agent.ipynb Attached is the landscape you will need to load into your google drive. or update the Per the dev session this morning I should be able to add a custom function to filter out non-traders or at least return None, but so far unsuccessful on that code. |
I think a temporary workaround is to put the trade partners in the model reporter instead.
I used |
This is indeed a (in my opinion big) limitation. I wrote a proof of concept implementation for it in #1142, and used it in a personal project. If anyone ones to built on it, feel free to use it in any way! |
While this looks more concise for Either:
We should look at how other ABM libraries choose their trade-off here. |
Could we create a new switch for the DataCollector, |
The maintenance effort is about the same with |
That's a good point. Would you be interested in creating an (draft) implementation? |
My plate is rather full right now. |
Thanks for the honest reply, let's discuss at the next development meeting how we can move this project forward. |
What if you would restructure the Maybe a Many of the code is now duplicate such as |
I'm also running into this issue with respect to the agent attributes. After reading through the code, I wonder if it would be possible to just pass a specific So something like this in the self.datacollector_my_agents = mesa.DataCollector(
schedule = self.schedule_my_agents, # arbitrary schedule with an .agents attribute that returns a list of agents
agent_reporters = {...},
model_reporters = {...}
)
self.datacollector_your_agents = mesa.DataCollector(
schedule = self.schedule_your_agents, # arbitrary schedule with an .agents attribute that returns a list of agents
agent_reporters = {...},
model_reporters = {...}
) Then to get the data: my_agent_data = model.datacollector_my_agents.get_agent_vars_dataframe()
your_agent_data = model.datacollector_your_agents.get_agent_vars_dataframe() |
This definitely could provide ingredients for people who want to build their own data collector class. But I don't see it to be cleaner than just inheriting from the current I think for now, for trading Sugarscape use case, we should implement the solution within the examples folder. Then wait for a while for it to become a part of the library. |
@jacob-thrackle your solution could work. Though we also want to make sure the >80% of the use case stays simple and convenient, where the scheduler is assigned to the model attribute |
Absolutely. I started hacking around with it last night but the following section makes it difficult because of the string type for Lines 156 to 166 in 373637e
If I'm understanding the code correctly, the condition at line 157 is only Lines 137 to 141 in 373637e
So having a reporter be a lambda or some other arbitrary function would make this not true and evaluate the Using the function has the added advantage of calling This is all to say that I was able to make my suggestion work by simply removing lines 157-161, only using the function if self.schedule:
schedule = self.schedule # new behavior
else:
schedule = model.schedule # old behavior, self.schedule defaults to None in the constructor This conditional has to be placed, unfortunately, in both |
That L157-161 seems to be written by @Corvince in aaa4c54. The original commit contains this comment for the # Fast path if all reporters are attribute collecters but had since been removed in the subsequent iterations. But the original PR that did the specialization of the code when all the reporters are string-based, is #576. #575 contains some benchmark results |
Thanks for digging that up @rht And yes, @jacob-thrackle you understood exactly right how it works. Is is probably better to give up a tiny bit of performance by using getattr instead of attrgetter to have default values |
…eudule and to return a default value of None if no such attribute exists.
…creation (defaults to model.schedule otherwise) and will return None if an attribute is not found instead of throwing an AttributeError.
@Corvince Cool, I opened a PR from my personal account (this is @jacob-thrackle). Let me know when you guys get around to reviewing it and what changes, if any, you'd like me to make. I forgot to link it in the commit but this also fixes #976. I made another version that kept the optimization but it only happened if two things were true: that |
…() now accepts a scheudle instead of a model. Attributes that do not exist return None instead of an AttributeError.
Describe the Feature
Receive attribute error when running an agent reporter through datacollector when using RandomActivationByType
Desired behavior
I can collect against different attributes based on agent type
To Reproduce
Create a model with different agent types and attributes and try to collect against that agent attribute
Additional context
Feature would need to update at line 170 of datacollection.py
agent_records = map(get_reports, model.schedule.agents)
The text was updated successfully, but these errors were encountered: