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

Require Mesa models to be initialized with super().__init__() #2218

Merged
merged 4 commits into from
Aug 18, 2024

Conversation

EwoutH
Copy link
Member

@EwoutH EwoutH commented Aug 17, 2024

Remove the workaround for when the model super has not been called, and thereby require Mesa models to be initialized with super().__init__().

class MyModel(mesa.Model):
    def __init__(self):
        super().__init__()  # This is now required!

This is needed to correctly add Agents to the model and setup other Mesa model variables.

This warning was present since Mesa 2.2.0 (#1928), so there has been some time to update for existing users.

This PR contains a second commit that names experimental warning in PropertyLayer correctly in space.py.

EwoutH added 2 commits August 17, 2024 14:34
Remove the workaround for when the model super has not been called, and thereby require Mesa models to be initialized with `super.__init__()`.

This warning was present since 2.3.0, so there has been some time to update for existing users.
The experimental_warning_given variable in PropertyLayer was named incorrectly, probably while copying from the AgentSet. This commit fixes that.
Copy link

Performance benchmarks:

Model Size Init time [95% CI] Run time [95% CI]
Schelling small 🔵 +0.7% [+0.3%, +1.1%] 🔵 -0.3% [-0.5%, -0.1%]
Schelling large 🔵 +1.4% [+0.9%, +1.9%] 🔵 -1.7% [-2.5%, -0.9%]
WolfSheep small 🔵 +0.0% [-1.3%, +1.2%] 🔵 -0.6% [-0.9%, -0.3%]
WolfSheep large 🔵 +0.7% [+0.5%, +1.0%] 🔵 -0.3% [-1.1%, +0.5%]
BoidFlockers small 🔵 +0.4% [-0.2%, +1.0%] 🔵 -1.0% [-1.7%, -0.1%]
BoidFlockers large 🔵 +1.0% [+0.3%, +1.7%] 🔵 -0.2% [-0.9%, +0.3%]

mesa/agent.py Outdated
FutureWarning,
stacklevel=2,
)
self.model.agents_[type(self)][self] = None
Copy link
Contributor

Choose a reason for hiding this comment

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

What about try-except then raise error, instead of a warning, for now? I'm not sure if some people do read the release note for such a subtle change in the way they have to do __init__().

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't think it's a bad idea, but with Mesa 3.0 I would like to clean the codebase and remove it altogether.

In the 3.0 release notes we can put a note to first update to the latest Mesa 2.x, which also contains this warning (and it now has been in there for all 2.2.x and 2.3.x releases).

Copy link
Member Author

Choose a reason for hiding this comment

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

Thought about it some more, and I think you're actually right. It's better to throw an useful error than let it crash without any guidance.

EwoutH added 2 commits August 18, 2024 09:55
You should always run super().__init__() after initializing your model class that inherits from Mesa Model.
Added `from err` to the `raise RuntimeError` line. This links the `RuntimeError` to the original `AttributeError`, making it clear that the new exception was raised because of the `AttributeError` caught in the `try` block.
@EwoutH EwoutH added the breaking Release notes label label Aug 18, 2024
@EwoutH EwoutH merged commit 90628fa into projectmesa:main Aug 18, 2024
10 of 12 checks passed
@EwoutH EwoutH mentioned this pull request Aug 21, 2024
7 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking Release notes label
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants