issue a user warning if random is None

[quaquel]

Adds a user warning to AgentSet, CellCollection, and DiscreteSpace if they are initialized without an explicit random number generator.

closes #2477

[quaquel]

The release/deploy action is failing with ModuleNotFoundError: No module named 'hatchling.dep' @EwoutH, any idea why?

[github-actions]

Performance benchmarks:

Model	Size	Init time [95% CI]	Run time [95% CI]
BoltzmannWealth	small	🔵 +0.5% [-0.2%, +1.1%]	🔵 +0.5% [+0.4%, +0.7%]
BoltzmannWealth	large	🔵 +0.3% [-0.1%, +0.6%]	🔵 +0.3% [+0.1%, +0.6%]
Schelling	small	🔵 +0.6% [+0.3%, +0.8%]	🔵 +1.2% [+0.9%, +1.4%]
Schelling	large	🔵 +0.7% [+0.3%, +1.1%]	🔵 +1.6% [+0.5%, +2.6%]
WolfSheep	small	🔵 +0.4% [+0.1%, +0.6%]	🔵 +0.1% [-0.1%, +0.2%]
WolfSheep	large	🔵 +0.0% [-0.6%, +0.6%]	🔵 +0.5% [-0.4%, +1.2%]
BoidFlockers	small	🔵 +0.5% [-0.1%, +1.1%]	🔵 -0.7% [-1.5%, -0.0%]
BoidFlockers	large	🔵 -0.1% [-0.8%, +0.5%]	🔵 +0.3% [-0.2%, +0.9%]

[EwoutH]

Looks unrelated to this PR, it happens also on the main branch.

[EwoutH]

Thanks!

The example models are being tested with werror, so there can't be warnings in them. From the CI it seems a few need to be updated.

[quaquel]

The example models are being tested with werror, so there can't be warnings in them. From the CI it seems a few need to be updated.

Which highlights the usefulness of these warnings!

[EwoutH]

Was just investigating the release build issue, as hatchling tagged a new build which fixed their broken build form this morning.

• ModuleNotFoundError: No module named 'hatchling.dep' with hatchling 1.26.0 pypa/hatch#1793

Reran the release job and it passes like it should.

[quaquel]

Any other comments? I suggest fixing examples once this is merged.

[EwoutH]

Sorry, bit of a miscommunication. I would like the examples updated in this PR (was waiting for that).

That's the nice thing now we have the examples in the main repo, we can directly show how it affects examples and the CI keeps working.

[quaquel]

Sorry, bit of a miscommunication. I would like the examples updated in this PR (was waiting for that).

That's the nice thing now we have the examples in the main repo, we can directly show how it affects examples and the CI keeps working.

I think you are mistaken. Below, I show the errors. These are examples in the examples repo, not those in the main repo. The examples in the main repo are part of the unit tests of the main repo.

==================================== PASSES ====================================
=========================== short test summary info ============================
PASSED test_examples.py::test_model_steps[GameOfLifeModel]
PASSED test_examples.py::test_model_steps[ElFarolBar]
FAILED test_examples.py::test_model_steps[ShapeExample] - UserWarning: Random number generator not specified, this can make models non-reproducible. Please pass a random number generator explicitly
FAILED test_examples.py::test_model_steps[Schelling] - UserWarning: Random number generator not specified, this can make models non-reproducible. Please pass a random number generator explicitly
FAILED test_examples.py::test_model_steps[BoltzmannWealthModelNetwork] - UserWarning: Random number generator not specified, this can make models non-reproducible. Please pass a random number generator explicitly
FAILED test_examples.py::test_model_steps[ColorPatches] - UserWarning: Random number generator not specified, this can make models non-reproducible. Please pass a random number generator explicitly
FAILED test_examples.py::test_model_steps[BankReservesModel] - UserWarning: Random number generator not specified, this can make models non-reproducible. Please pass a random number generator explicitly
FAILED test_examples.py::test_model_steps[Charts] - UserWarning: Random number generator not specified, this can make models non-reproducible. Please pass a random number generator explicitly
FAILED test_examples.py::test_model_steps[AcoTspModel] - UserWarning: Random number generator not specified, this can make models non-reproducible. Please pass a random number generator explicitly
FAILED test_examples.py::test_model_steps[ForestFire] - UserWarning: Random number generator not specified, this can make models non-reproducible. Please pass a random number generator explicitly
FAILED test_examples.py::test_model_steps[HotellingModel] - UserWarning: Random number generator not specified, this can make models non-reproducible. Please pass a random number generator explicitly
FAILED test_examples.py::test_model_steps[HexSnowflake] - UserWarning: Random number generator not specified, this can make models non-reproducible. Please pass a random number generator explicitly
=================== 10 failed, 2 passed, 1 warning in 8.71s ====================

[EwoutH]

You're right, my bad.

We made a slight regression in example testing, because the unittest weren't running with werror, and when the example moved they inherited that. I would like to keep our examples warning-free, so could you resolve these warnings?

=============================== warnings summary ===============================
tests/test_cell_space.py: 13 warnings
tests/test_examples.py: 2 warnings
  /home/runner/work/mesa/mesa/mesa/experimental/cell_space/grid.py:57: UserWarning: Random number generator not specified, this can make models non-reproducible. Please pass a random number generator explicitly
    super().__init__(capacity=capacity, random=random, cell_klass=cell_klass)

tests/test_cell_space.py: 16 warnings
tests/test_components_matplotlib.py: 6 warnings
tests/test_examples.py: 4 warnings
tests/test_solara_viz.py: 1 warning
  /home/runner/work/mesa/mesa/mesa/experimental/cell_space/discrete_space.py:76: UserWarning: Random number generator not specified, this can make models non-reproducible. Please pass a random number generator explicitly
    return CellCollection({cell: cell.agents for cell in self._cells.values()})

tests/test_cell_space.py::test_networkgrid
tests/test_cell_space.py::test_empties_space
tests/test_cell_space.py::test_agents_property
tests/test_cell_space.py::test_copying_discrete_spaces
  /home/runner/work/mesa/mesa/mesa/experimental/cell_space/network.py:29: UserWarning: Random number generator not specified, this can make models non-reproducible. Please pass a random number generator explicitly
    super().__init__(capacity=capacity, random=random, cell_klass=cell_klass)

tests/test_cell_space.py::test_voronoigrid
tests/test_solara_viz.py::test_call_space_drawer
  /home/runner/work/mesa/mesa/mesa/experimental/cell_space/voronoi.py:193: UserWarning: Random number generator not specified, this can make models non-reproducible. Please pass a random number generator explicitly
    super().__init__(capacity=capacity, random=random, cell_klass=cell_klass)

  /home/runner/work/mesa/mesa/mesa/experimental/cell_space/cell_collection.py:124: UserWarning: Random number generator not specified, this can make models non-reproducible. Please pass a random number generator explicitly
    return CellCollection(cell_generator(filter_func, at_most))

I will look into if we can better configure this in the CI - and if other warnings have creeped in.

[Corvince]

I am not 100% convinced we need a warning here. In my experience people receive warnings very differently. I, for example, am always a bit scared by them and have the feeling something is wrong.
Not sure this is necessarily the case here. Yes, for scientific work, models should definitely be reproducible. But for toy models or teaching I am not so sure. It's definitely best practice, but also adds cognitive load. Maybe an Info instead of a warning could be raised?

[quaquel]

Maybe an Info instead of a warning could be raised?

I am not sure what you are suggesting. If this were log messages, then yes it would make sense to issue an INFO level log message. However, I am using the warnings library here, so it will always be some kind of warning, unless I miss something obvious?

[quaquel]

We made a slight regression in example testing, because the unittest weren't running with werror, and when the example moved they inherited that. I would like to keep our examples warning-free, so could you resolve these warnings?

Not everything listed there is examples related. I have now fixed the examples so they all correctly instantiate a new-style space with an explicit random number generator. I also fixed all cell_space related unittests to longer issue the warnings added in this PR. Do you want me to fix the mesa.visualization related tests as well?

as an aside: it might be worth reviewing the property layer user warning. I have no idea how to get rid of that one even in my own code.

[EwoutH]

What if we let users pass a certain value to random explicitly state it should be non-reproduceable, to suppress the warning? Something like:

def __init__(self, random: Union[Random, bool, None] = None):
    if random is None:
        warnings.warn(
            "Random number generator not specified, this can make models non-reproducible. "
            "Please pass a random number generator explicitly to make it reproducible, or pass random=True to create a new random number generator."
            UserWarning,
            stacklevel=2,
        )
        self.random = Random()
    elif random is True:
        self.random = Random()  # Create new RNG without warning
    else:
        self.random = random

[Corvince]

What if we let users pass a certain value to random explicitly state it should be non-reproduceable, to suppress the warning? Something like:

I don't think thats a common use case. I was thinking more about just ignoring the argument. If you already make a choice about what to pass to "random", you can just pass a random generator.

But @quaquel is right, there is no such functionality as I imagined. I am sure I saw some "info" messages before, but then it was probably just some regular "print" calls formatted nicely. So I would say just leave it as it is now in this PR.

[EwoutH]

A lot of users are going to encounter this warning, so I really want is documented well. It should be clear:

• Where the warning originates from (multiple locations are possible I see)
• Why it's there
• How the user can resolve it

I think that's too much for one warning message, so maybe we can include a nice write up somewhere, and then link to it in the error message

[quaquel]

A lot of users are going to encounter this warning, so I really want is documented well. It should be clear:

• Where the warning originates from (multiple locations are possible I see)
• Why it's there
• How the user can resolve it

It can originate in three places: DiscreteSpace, AgentSet, or Cellcollection. Of thsee, 95% of users will encounter it with DiscreteSpace. It is uncommon to instantiate the other classes directly as a user. In fact, we don't do so in any of our example models.

The simple solution is to just add simple note to the docstring of these three classes. Something like

"""
note: A `UserWarning` is issued if `random=None`. You can resolve this warning by explicitly passing a random number generator. In most cases, this will be the seeded random number generator in the model. So, you would do `random=self.random` in a `Model` or `Agent` instance.
"""

[EwoutH]

It is uncommon to instantiate the other classes directly as a user. In fact, we don't do so in any of our example models.

Okay, that helps a lot. So it's mainly for the discrete spaces. In that case I'm fine with just some added docstring.

[EwoutH]

Aside from the docstring, would this warning message make it even clearer?

warnings.warn(
    f"Random number generator not specified while initializing {self.__class__.__name__}. "
    "This can make models non-reproducible. To fix this, pass the model's random "
    "number generator (e.g., random=self.random from within Model or Agent classes)",
    UserWarning,
    stacklevel=2,
)

[quaquel]

The latter part could be added, the first part about which class will be clear from the warning context