Fix sliding window eviction crash in ModelDataSet by ShreyasN707 · Pull Request #3389 · mesa/mesa

ShreyasN707 · 2026-02-26T17:37:30Z

Fixes a crash in ModelDataSet when window_size is enabled by standardizing how blocks are stored internally.

Issue: #3378

When the sliding window eviction runs, _evict_blocks expects blocks in the format (time, data).
However, dict-based data was being stored as a merged dictionary ({"time": time, **data}), which caused a ValueError during unpacking and crashed the data collection process.

Implementation

Unified the storage format so all blocks (including dict data) are stored as (time, data) tuples.
The merge into {"time": time, **data} is now handled only when building the final DataFrame.

Core change:

# datarecorders.py

case dict():
    storage.blocks.append((time, data))
    storage.total_rows += 1
rows = [{**data, "time": time} for time, data in storage.blocks]
return pd.DataFrame(rows)

github-actions · 2026-02-26T17:53:57Z

Performance benchmarks:

Model	Size	Init time [95% CI]	Run time [95% CI]
BoltzmannWealth	small	🔵 -2.0% [-2.7%, -1.2%]	🟢 -4.2% [-4.6%, -3.8%]
BoltzmannWealth	large	🔵 -0.0% [-0.5%, +0.5%]	🔵 +1.3% [-0.3%, +3.2%]
Schelling	small	🔵 +0.5% [-0.1%, +1.0%]	🔵 +1.8% [+1.6%, +1.9%]
Schelling	large	🔵 -1.1% [-2.0%, -0.1%]	🔵 -2.0% [-4.4%, +0.5%]
WolfSheep	small	🟢 -4.9% [-5.8%, -4.1%]	🔵 -2.2% [-2.6%, -1.8%]
WolfSheep	large	🟢 -4.8% [-6.4%, -3.1%]	🟢 -8.7% [-10.9%, -6.4%]
BoidFlockers	small	🔵 -0.8% [-1.1%, -0.5%]	🔵 -0.7% [-0.9%, -0.6%]
BoidFlockers	large	🔵 -0.0% [-0.6%, +0.4%]	🔵 -0.2% [-0.5%, -0.0%]

tests/experimental/test_modeldataset_window.py

ShreyasN707 · 2026-02-27T18:11:32Z

@EwoutH Tests are passing now — updated the dict storage to (time, data).

quaquel · 2026-02-28T06:55:07Z

@codebreaker32 could you take a look at this?

codebreaker32

Good catch and thanks for this PR @ShreyasN707 However I have some minor requests:

Please remove "time" from the storage.metadata["columns"] as well, keeping it in the metadata columns will cause empty DataFrames to have an inconsistent schema.
Could you add a specific test that explicitly guards against the ValueError crash?

I will give it a final look once you push the changes

codebreaker32 · 2026-03-01T05:51:25Z

mesa/experimental/data_collection/datarecorders.py

                    storage.metadata["type"] = "custom"

-        # Update bookkeeping for evicted data
+        """Update bookkeeping for evicted data"""


Please don't use triple quotes for inline comments

codebreaker32 · 2026-03-01T05:51:45Z

mesa/experimental/data_collection/datarecorders.py


        if not storage.blocks:
-            # Empty DataFrame with correct columns
+            """ Empty DataFrame with correct columns"""


Please don't use triple quotes for inline comments

codebreaker32 · 2026-03-01T05:51:57Z

mesa/experimental/data_collection/datarecorders.py

                return self._convert_modelDataSet(storage)
            case _:
-                # Fallback
+                """Fallback"""


Same as above

codebreaker32 · 2026-03-01T05:52:41Z

tests/experimental/test_datarecorder.py


-def test_data_recorder_window_eviction_dict():
-    """Test window eviction bookkeeping for dict data."""
+def test_modeldataset_window_eviction_crash():


Why did you change the name?

tests/experimental/test_datarecorder.py

codebreaker32 · 2026-03-01T05:53:45Z

tests/experimental/test_datarecorder.py

-    # Fill window
    recorder.collect()
    recorder.collect()
-    recorder.collect()  # Should evict first


Same as above

Looks like I removed a few comments unintentionally during conflict resolution. My bad I’ll correct that and update the PR.

codebreaker32 · 2026-03-01T06:02:49Z

I still couldn't see the test that explicitly guards against the ValueError mentioned in issue

ShreyasN707 · 2026-03-01T06:37:29Z

I still couldn't see the test that explicitly guards against the ValueError mentioned in issue

I’ll recheck the branch it’s possible the test didn’t get pushed properly. I’ll confirm and fix it right away.

codebreaker32 · 2026-03-01T19:02:09Z

mesa/experimental/data_collection/datarecorders.py

+            if "time" not in columns:
+                columns = [*columns, "time"]


Yes adding "time" there was unnecessary; I'll remove it to keep the schema aligned with metadata-defined columns.

codebreaker32 · 2026-03-01T19:02:32Z

mesa/experimental/data_collection/datarecorders.py

        """Convert model dict blocks to DataFrame."""
        if not storage.blocks:
-            return pd.DataFrame(columns=storage.metadata.get("columns", []))
+            return pd.DataFrame(columns=[*storage.metadata.get("columns", []), "time"])


codebreaker32 · 2026-03-01T19:04:25Z

tests/experimental/test_datarecorder.py

+def test_modeldataset_window_eviction_no_valueerror():
+    """Explicit regression test for ValueError during sliding window eviction (#3378)."""


It seems repetitive to me. Isn't it similar to above test? I am fine with above tests only

It overlapped with the existing dict eviction test. The only difference was an explicit try/except for the ValueError, but pytest already fails on uncaught exceptions, so it was redundant.

…age format

for more information, see https://pre-commit.ci

codebreaker32

LGTM! Thanks

quaquel · 2026-03-02T17:29:01Z

tests are failing, so that needs to be addressed before merging this PR.

mesa/experimental/data_collection/datarecorders.py

Co-authored-by: Aman Bihari <[email protected]>

Nithurshen reviewed Feb 26, 2026

View reviewed changes

tests/experimental/test_modeldataset_window.py Outdated Show resolved Hide resolved

ShreyasN707 force-pushed the fix/modeldataset-window-eviction branch 3 times, most recently from b3da951 to 4a3b3ff Compare February 27, 2026 17:33

codebreaker32 reviewed Feb 28, 2026

View reviewed changes

ShreyasN707 force-pushed the fix/modeldataset-window-eviction branch from 01dddc0 to d4cd210 Compare March 1, 2026 04:37

ShreyasN707 requested a review from codebreaker32 March 1, 2026 04:39

ShreyasN707 force-pushed the fix/modeldataset-window-eviction branch from 6c05284 to 0985233 Compare March 1, 2026 04:49

codebreaker32 reviewed Mar 1, 2026

View reviewed changes

ShreyasN707 force-pushed the fix/modeldataset-window-eviction branch from d364219 to 32654d6 Compare March 1, 2026 06:53

ShreyasN707 requested a review from codebreaker32 March 1, 2026 18:03

codebreaker32 reviewed Mar 1, 2026

View reviewed changes

codebreaker32 mentioned this pull request Mar 1, 2026

Add time column to empty dataframe in datarecorder #3408

Merged

ShreyasN707 added 4 commits March 2, 2026 15:43

Fix ModelDataSet sliding window eviction crash by unifying block stor…

b79fc14

…age format

restored the comments

424799b

removed the duplication test

d5f5016

removed left over commit marker

389e1db

ShreyasN707 force-pushed the fix/modeldataset-window-eviction branch from 6683a8f to 389e1db Compare March 2, 2026 10:17

removed manual adding of time

805384a

ShreyasN707 force-pushed the fix/modeldataset-window-eviction branch from d1221d9 to 805384a Compare March 2, 2026 10:22

pre-commit-ci bot and others added 4 commits March 2, 2026 10:23

[pre-commit.ci] auto fixes from pre-commit.com hooks

fd4c394

for more information, see https://pre-commit.ci

fixed comments caused due to conflicts

6ee1ad7

[pre-commit.ci] auto fixes from pre-commit.com hooks

ebf5d83

for more information, see https://pre-commit.ci

Merge branch 'main' into fix/modeldataset-window-eviction

06468ba

ShreyasN707 requested a review from codebreaker32 March 2, 2026 16:24

codebreaker32 approved these changes Mar 2, 2026

View reviewed changes

Merge branch 'main' into fix/modeldataset-window-eviction

87cea47

Merge branch 'main' into fix/modeldataset-window-eviction

24502db

codebreaker32 reviewed Mar 2, 2026

View reviewed changes

mesa/experimental/data_collection/datarecorders.py Outdated Show resolved Hide resolved

ShreyasN707 and others added 3 commits March 3, 2026 14:15

Merge branch 'main' into fix/modeldataset-window-eviction

4167c54

suggested changes

06cc724

Co-authored-by: Aman Bihari <[email protected]>

Merge branch 'main' into fix/modeldataset-window-eviction

a066f8b

quaquel approved these changes Mar 3, 2026

View reviewed changes

quaquel merged commit aa3787e into mesa:main Mar 3, 2026
11 of 12 checks passed

EwoutH added bug Release notes label experimental Release notes label labels Mar 13, 2026

		def test_modeldataset_window_eviction_no_valueerror():
		"""Explicit regression test for ValueError during sliding window eviction (#3378)."""

Uh oh!

Conversation

ShreyasN707 commented Feb 26, 2026

Issue: #3378

Implementation

Uh oh!

github-actions bot commented Feb 26, 2026

Uh oh!

Uh oh!

ShreyasN707 commented Feb 27, 2026

Uh oh!

quaquel commented Feb 28, 2026

Uh oh!

codebreaker32 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

codebreaker32 commented Mar 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ShreyasN707 commented Mar 1, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

codebreaker32 left a comment

Choose a reason for hiding this comment

Uh oh!

quaquel commented Mar 2, 2026

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

codebreaker32 commented Mar 1, 2026 •

edited

Loading