Skip to content

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: https://github.com/projectmesa/mesa-examples/blob/cf38477725771d4658ea6dcf393766f1feefcc88/examples/sugarscape_g1mt/sugarscape_g1mt/model.py#L188-L205.

@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 mesa:main Jan 24, 2024
@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