Explicit fixed and variable parameters for batchrunner #374
Conversation
- Some common functionality factored out into separate method - New mock model for test suit - A default list argument in StagedActivation init replaced with None
Make batch runner variable parameters more explicit and robust
|
This all looks good, except for the example models need to be updated...
I think that is all of them. |
|
Good point! I'll work on that today.
…On Tue, May 23, 2017, 11:01 Jackie Kazil ***@***.***> wrote:
This all looks good, except for the example models need to be updated...
- mesa/examples/schelling notebook
- mesa/examples/forest_fire notebook
- mesa/docs/tutorials intro notebook
I *think* that is all of them.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#374 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/ASzauCfYP6cO1ZYv9iVvwInQrFirVEPmks5r8x8RgaJpZM4NjAfk>
.
|
|
@njvrzm I can tackle updating the examples if that would be helpful. |
| def _process_parameters(self, params): | ||
| params = copy.deepcopy(params) | ||
| bad_names = [] | ||
| for name, values in params.items(): |
There was a problem hiding this comment.
This typecheck can be enforced with PEP484, will be much simpler. The version requirement will have to be bumped to be at least 3.5.
| self.parameter_values = {param: self.make_iterable(vals) | ||
| for param, vals in parameter_values.items()} | ||
| self.variable_parameters = self._process_parameters(variable_parameters) | ||
| self.fixed_parameters = fixed_parameters or {} |
There was a problem hiding this comment.
This reads like fixed_parameters || {} ;) -- {} should have been a default value for fixed_parameters, instead of None.
There was a problem hiding this comment.
Oh, this absolutely a no-no in Python. Never use mutable objects as default parameters - they're shared between every invocation of the function, so if they're modified, terrible bugs can creep in. The way this works is quite standard.
There was a problem hiding this comment.
This is pylint w0102[1].
| total_iterations *= len(param_range) | ||
| with tqdm(total_iterations, disable=not self.display_progress) as pbar: | ||
| for param_values in product(*param_ranges): | ||
| kwargs = dict(zip(param_names, param_values)) |
There was a problem hiding this comment.
This is basically kwargs = self.variable_parameters
mesa/batchrunner.py
Outdated
| bad_names = [] | ||
| for name, values in params.items(): | ||
| if (isinstance(values, str) or | ||
| not isinstance(values, collections.Sequence)): |
There was a problem hiding this comment.
This fails with numpy sequences (e.g. in forest_fire/Forest Fire Model.ipynb)
There was a problem hiding this comment.
I found an unresolved issue, but with a patch here numpy/numpy#2776 (comment). Sequence.register(np.ndarray) has to be added at somewhere.
There was a problem hiding this comment.
I think it's also resolved with hasattr(values, "__iter__")
Updating the examples and documentation, and fixing the bug that disregarded numpy sequences.
|
It looks like the tests are failing due to changes in the visualization API. Can someone take a look and confirm / verify, and see whether it's something that needs to be addressed here, or will self-resolve with the merge? |
|
It has to be addressed here, the error in https://travis-ci.org/projectmesa/mesa/jobs/249398056#L1038 |
|
It is because mesa/tests/test_batchrunner.py Line 17 in bce8322 MockModel has to be inserted before val. reset_model in ModularServer needs to be modified for this.
|
|
In addition to having that bug fixed, the branch also needs to be rebased. |
|
Heads up: to reduce the number of roundtrips, I have fixed the errs, rebased in #393. |
A fix for #307. The batchrunner was getting confused if there was a parameter with a single value that was an iterable type, thinking it was getting a range of values instead. This change provides for explicit variable parameters, which take a sequence of values, and fixed parameters, which take a single value (which may itself be a sequence type).