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

Fixed batch_run accepting empty iterables #2155

Closed
wants to merge 2 commits into from

Conversation

its-nmt05
Copy link

Description

This PR addresses an issue in the batch_run method where parameters must be iterable and non-empty.

Changes

Added iterable and non-empty checks in _make_model_kwargs.
Updated batch_run to handle only valid parameters.

Issue Reference

Closes #2108

Copy link

Performance benchmarks:

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

@its-nmt05 its-nmt05 force-pushed the its-nmt05/fix-batch-run branch from 88afb09 to e2866f5 Compare June 22, 2024 18:04
@rht
Copy link
Contributor

rht commented Jun 24, 2024

@its-nmt05 thank you for your contribution. Upon testing via the bank reserves example, I got this error

Traceback (most recent call last):
  File "/projectmesa/mesa-examples/examples/bank_reserves/batch_run.py", line 190, in <module>
    data = mesa.batch_run(
           ^^^^^^^^^^^^^^^
  File "/projectmesa/mesa/mesa/batchrunner.py", line 67, in batch_run
    data = process_func(run)
           ^^^^^^^^^^^^^^^^^
  File "/projectmesa/mesa/mesa/batchrunner.py", line 138, in _model_run_func
    model = model_cls(**kwargs)
            ^^^^^^^^^^^^^^^^^^^
  File "/projectmesa/mesa-examples/examples/bank_reserves/batch_run.py", line 157, in __init__
    for i in range(self.init_people):
             ^^^^^^^^^^^^^^^^^^^^^^^
TypeError: 'list' object cannot be interpreted as an integer

@rht
Copy link
Contributor

rht commented Jun 24, 2024

It's because I intentionally set init_people to []. I think the correct handling should be to raise error instead of silently passing along the values. In the error message, you should say that if the param values is empty, it should be dropped altogether by the user by hand.

@EwoutH
Copy link
Member

EwoutH commented Jul 3, 2024

Yeah I agree, explicit is better than implicit.

We should also document which values we allow and which we don't (like an empty list or not).

@quaquel
Copy link
Member

quaquel commented Dec 5, 2024

superseded by #2523

@quaquel quaquel closed this Dec 5, 2024
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.

Batch_Run begins, performs no iterations, no errors, writes empty .csv file. Help?
4 participants