Skip to content
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

fix: Ensure schedule.steps is always incremented before data collection #93

Merged
merged 1 commit into from
Jan 24, 2024

Conversation

rht
Copy link
Contributor

@rht rht commented Jan 24, 2024

This replaces #92.

@EwoutH
Copy link
Member

EwoutH commented Jan 24, 2024

Could you describe the problem and why this solution fixed it, for clarity?

@rht
Copy link
Contributor Author

rht commented Jan 24, 2024

The problem is when trade is disabled, the steps never get incremented because the code exits early in this code branch. Needs a manual self.schedule.steps += 1, because there is no self.schedule.step() call. The trading Sugarscape needs custom scheduling that is beyond staged activation, and is more easily expressed by the AgentSet API.

@EwoutH
Copy link
Member

EwoutH commented Jan 24, 2024

Would this work? If so, I think it's more elegant:

def step(self):
    """
    Unique step function that does staged activation of sugar and spice
    and then randomly activates traders
    """
    # Step Resource agents
    for resource in self.schedule.agents_by_type[Resource]:
        resource.step()

    # Step trader agents
    trader_shuffle = self.randomize_traders()
    for agent in trader_shuffle:
        agent.prices = []
        agent.trade_partners = []
        agent.move()
        agent.eat()
        agent.maybe_die()

    if self.enable_trade:
        # If trade is enabled, shuffle and proceed with trading
        trader_shuffle = self.randomize_traders()
        for agent in trader_shuffle:
            agent.trade_with_neighbors()

    # Update step count and collect data
    self.schedule.steps += 1
    self.datacollector.collect(self)

@rht
Copy link
Contributor Author

rht commented Jan 24, 2024

No it doesn't, because there are more lines of code beyond self.datacollector.collect(self) when trade is enabled:

"""
Mesa is working on updating datacollector agent reporter
so it can collect information on specific agents from
mesa.time.RandomActivationByType.
Please see issue #1419 at
https://github.com/projectmesa/mesa/issues/1419
(contributions welcome)
Below is one way to update agent_records to get specific Trader agent data
"""
# Need to remove excess data
# Create local variable to store trade data
agent_trades = self.datacollector._agent_records[self.schedule.steps]
# Get rid of all None to reduce data storage needs
agent_trades = [agent for agent in agent_trades if agent[2] is not None]
# Reassign the dictionary value with lean trade data
self.datacollector._agent_records[self.schedule.steps] = agent_trades
.

@rht
Copy link
Contributor Author

rht commented Jan 24, 2024

We can optimize the code clarity later. This PR's scope is to fix the test issue with a very localized patch.

Copy link
Member

@EwoutH EwoutH left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indeed, this PR fixes the issue. We can refactor later.

@rht rht merged commit 479eaf3 into projectmesa:main Jan 24, 2024
2 of 3 checks passed
@rht rht deleted the g1mt branch January 24, 2024 15:54
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.

2 participants