-
Notifications
You must be signed in to change notification settings - Fork 925
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
model: Automatically increase steps
counter
#2223
Conversation
Performance benchmarks:
|
mesa/model.py
Outdated
@@ -77,6 +77,20 @@ def __init__(self, *args: Any, **kwargs: Any) -> None: | |||
self._steps: int = 0 | |||
self._time: TimeT = 0 # the model's clock | |||
|
|||
# Wrap the user-defined step method | |||
if hasattr(self, "step"): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What if the user defines their step with a different name, e.g. in the simultaneous activation?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good edge case! I think when using a scheduler, you're also mainly using the scheduler's time, so model.schedule.time
. And as long as one of the functions is a step()
, the model time will also be increased.
Personally I'm in favor of moving away from the schedulers, but this PR won't break that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can't leave up to chance. If the model doesn't have a step, it is more informative to raise an error with an informative message.
Alternatively, bumping the step & time whenever model.agents.do
is called, is a possible option.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If the model doesn't have a step, it is more informative to raise an error with an informative message.
If you want to use your own process without using Mesa's internal clock, it's perfectly valid. I think we can cover this with documentation, and this will be less and less a problem as we move away from schedulers.
I added an example of staged time increase to the PR start.
Alternatively, bumping the step & time whenever model.agents.do is called, is another alternative.
The problem is, how do you handle multiple agents.do calls? Or agents.do to a partial agentset? Or with a select in between?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Alternatively, bumping the step & time whenever model.agents.do is called, is another alternative.
The problem is, how do you handle multiple agents.do calls? Or agents.do to a partial agentset? Or with a select in between?
If we want to track the number of steps, it should be tied to model.step
. It's a bad idea to tie it to the AgentSet. Conceptually, this would mix up two completely different things. Which, as @EwoutH indicates, gives rise to all kinds of knock-on problems.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you want to use your own process without using Mesa's internal clock, it's perfectly valid. I think we can cover this with documentation, and this will be less and less a problem as we move away from schedulers.
steps
's need is hardcoded it the batch runner and visualization. Currently, they are not optional.
I'll try to look at the code later today, but just some quick first questions/thoughts/concerns.
|
I think both can be useful in some cases. Steps are just the number of times For example, if I have a model that runs from 8 o'clock in the morning to 22 o'clock in the evening, with 5 minute steps, I might do: class MyModel(Model):
def __init__(self):
...
self._time = 8
def step(self, time=1/12):
... Then I have a human-readable value of the time of day, while the steps can be useful for plotting, datacollection and debugging. So one is human readable and one is an uninterrupted sequence of integers. Of course, we could aim our documentation and tutorials to mainly use one.
In my opinion, a model should be runnable without too many additional fluff. A simulator or scheduler should not be required for a simple ABM (and projectmesa/mesa-examples#161 proves those aren't needed for the vast majority of models). |
Fair enough, but then I would keep support for tracking time / the number of ticks also minimal. What about having full time support in the simulator and only have number of ticks/steps in the model class? |
To do stages / simultaneous activation it can be nice to separate steps and time (see example in PR start). Also steps is just a counter and time can be used for a lot more. Maybe the experimental |
No, |
I have been thinking about this PR a bit more. I think having an automatic counter for the number of times However, I would keep the other stuff like time, running automatically for a given number of steps, etc. for the Splitting the functionality between tracking the number of times steps is called in the model and having a separate class for more advanced time management is my preferred way forward. This makes it possible for a user to keep building simple models as currently done (via a |
Thanks for your insights. I will be thinking about them a bit more, but for now, I see it this way:
The Looking at code complexity, adding and tracking a So, my current standpoint:
Edit: We might allow time to be a full Datetime object. Can imagine some scenarios in which that would be very handy. |
|
I didn't want to run too far ahead, but I was thinking of allowing the index of the DataCollector to be changeable. Default could be And maybe the same for visualisation. (with fixed to 1, I meant to fix it to increase always by 1) |
Why skip it or increase it in a non-fixed way? from a modeling point of view, this makes no sense to me. If "for readability", why should this be supported by MESA?
The ABMSimulator can indeed be done by DEVSSimulator as well. The main use case for me is fixed time advancement (so increment by 1 and automagically calls model.step each tick) while having integer-based event scheduling. Conceptually, it is a real devs-ABM hybrid. |
You can do cute stuff like conditionally modifying your time resolution, to perform more ticks at certain moments: def step(self, time=False):
# Increase volatility and decrease time increment during opening and closing hours
if 9 <= self._time < 10 or 15 <= self._time < 16:
self.volatility = 0.03
self._time += timedelta(minutes=5)
else:
self.volatility = 0.01
self._time += timedelta(minutes=30)
# Update stock prices
for stock in self.agents:
stock.update_price(self.volatility) If you allow datetime, you can even go a bit further: # If we've passed 4:00 PM, move to 9:00 AM the next day
if self._time >= 16:
next_day = self._time.date() + timedelta(days=1)
self._time = datetime.combine(next_day, datetime.min.time()) + timedelta(hours=9) And imagine you have external data lookups that use the time. Or in your data collector you can aggerate by hour, while still having more resolution available. It's basically just an additional built-in counter you can use before having to jump immediately to discrete even scheduling. |
That's not cute stuff, but bad modeling. It breaks discrete time advancement, which is a core principle behind ABMs. If a model requires variable time steps, you should use DEVS and carefully check all code to ensure time units are handled correctly. Also, and separately, is this use case so common that it should be supported in a core class of MESA? |
If you looked at the PR's timeline, the time attribute was added months before the DEVS PR https://github.com/projectmesa/mesa/pulls?q=is%3Apr+discrete+event+is%3Aclosed. At the time, it was the only official way of having time being tracked. But again, even if the DEVS did not exist, the time attribute doesn't necessarily have to be there by default: for users who need it, they can always define their time attribute as needed, and it would need only a few lines of code for the feature. |
And that's why it's not the default. I agree that it's not bad practice for most cases. But if an users finds a novel case or use, why deny the flexibility? I'm going to summarize my points:
|
I think my main objection is not to time attribute perse but to the use of time and step as optional keyword arguments to model.step. This is a new feature, might break existing models, and gives rise to a lot of the problems I have been alluding to. If users want to do something like this in their custom models, they can do so. But why support it within MESA itself given the conceptual issues I have been highlighting? Staged activation has its own internal time attribute. Which is conceptually strange. As argued before, there should be a single truth for time. I have to think about substeps with staged activation. I am unsure about how to handle that properly but seperating step and time might indeed be defendable for this. |
Would removing the Fun fact: I have a DEVS model in development right now where a steps counter would be very useful, since I still have a model step and want to do something once every N steps, while the remainder of the model is DEVS controlled. |
Wait there is one use case we haven't discussed: Some people might want to step at another place in the step, than only at the beginning of the step. Thereful it would be useful to turn the automatic step increase of. We could make the But in general, I'm for a flexible library with sensible defaults. |
use devs_simulator.time? or swicht to the ABMSimulator and use its time attribute should fix this for you.
why? I think with clear documentation (so allways at the start or allways at the end) any use case can easily be accomodated. |
I think at this point we disagree on a philosophical level what a simulation library should offer. @projectmesa/maintainers, I would love some fresh perspectives. |
As an alternative example, in our agents-and-networks gis example, we keep a clock manually : https://github.com/projectmesa/mesa-examples/blob/0ebc4d11711eb59480942d24dd13e9a744744704/gis/agents_and_networks/src/model/model.py#L215
This default behavior seems a bit redundant to me. TBH I wasn't really aware of the If a clock is really really needed, we probably need something more dedicated, like what AnyLogic offers: https://www.anylogic.com/upload/books/new-big-book/16-model-time-date-and-calendar.pdf which is also mentioned above:
But this may not be needed by all models, especially those simple models. So it may be better to be optional. |
Let me be perfectly clear: It's 100% optional. You can just do
That's quite interesting. It supports the need for a separate clock/time aside from |
My last question was really about trying to understand the issue. So not philosopical at al. In line with @wang-boyu, however, I am in favor of simply counting steps and leave detailed time management to an optional more advanced feature (possibly to be integrated into the simulator classes which offer much of what is described in the book chapter). |
But that means the |
Yeah on that point I incline to agree with you, I don't see many scenarios where you want to increase the step counter at another point in the step. What I meant with philosophical differences, is that I think convention over configuration is in general a good principle for Mesa to follow. Why? Because it offers two things that perfectly fits our mixed target audience.
And are there use cases for this specific hook? In my opinion, yes:
|
The whole |
instead of model._steps
a9575ec
to
80e32d1
Compare
step
counter
I believe this is ready for final review. I would like to merge myself. |
Performance benchmarks:
|
- Add a few tests - Check that for each example model.steps == 10
bca99d0
to
95e1cd2
Compare
There we go. Mesa is now incrementing time automatically. After these two weeks I can conclude this effort with the honor of writing the 65st message in this PR (of course not counting all the discussions outside). Like a good friend of mine once said: "De rijst is de bestelling" 🍋 |
step
countersteps
counter
This commit reverts PR #161 projectmesa/mesa-examples#161 That PR assumed that time advancement would be done automatically, like proposed in #2223 We encountered some underlying issues with time, which we couldn't resolve in time.
This commit reverts PR #161 projectmesa/mesa-examples#161 That PR assumed that time advancement would be done automatically, like proposed in #2223 We encountered some underlying issues with time, which we couldn't resolve in time.
…lity (projectmesa#170) This commit reverts PR projectmesa#161 projectmesa/mesa-examples#161 That PR assumed that time advancement would be done automatically, like proposed in projectmesa#2223 We encountered some underlying issues with time, which we couldn't resolve in time.
…lity (projectmesa#170) This commit reverts PR projectmesa#161 projectmesa/mesa-examples#161 That PR assumed that time advancement would be done automatically, like proposed in projectmesa#2223 We encountered some underlying issues with time, which we couldn't resolve in time.
This pull request adds automatically increasing of
steps
counter in the Mesa model, even if the users overwrites theModel.step()
method. It also removes the_time
variable.Background
Previously, Mesa required users to manually increment the step counters using the
_advance_time()
method, or by modifying themodel._time
andmodel._step
values directly. This use of an private method was non-ideal.Changes
steps
is now incremented automatically within theModel
class itself. Each call to thestep()
method will increment thesteps
counter by 1.The
_time
counter is removed (for motivation, read the discussion below).Implementation details
The core change involves wrapping the user-defined
step
method with_wrapped_step
which handles the incrementing process before executing the user's step logic. This ensures that all increment operations are centrally managed and transparent to the user.I choose to increment the time before the user step, in the spirit of "a new step" (or a new day) has begun. Then the model does things during the step, and.
Here's the updated version of your examples with the initial whitespaces removed from the code blocks:
Example Usage
You don't need to do anything to use this features. You can access steps at any moment.
Todo:
Check how this interacts with the experimental DEVS simulatorResolve:
steps
is automatically increased by exactly1
._time
is removed.steps
. Further discussion can continue.