Add sample and choices to CellCollection

[quaquel]

This adds 2 new methods to cell collection: sample_random_cells and choices_random_cells. These methods make it possible to sample cells with or without replacement from a cell collection. This closes #2530.

Implementation-wise, the methods are CellCollection specific wrappers around random.sample and random.choices. Note also that both methods return a list. A CellCollection cannot contain duplicates (it is, in essence, a fixed set). But both sampling and choices can return duplicates, so we cannot return a cell collection.

The main use case for these methods is in conjunction with the Agent.create_agents. You can. now do

class MyAgent(CellAgent):
    def __init__(self, model, cell):
        super().__init__(model)
        self.cell = cell

# let's create 10 agents, placed in 10 random empty cells
MyAgent.create_agents(model, 10, self.space.empties.sample_random_cells(k=10))

# let's create 10 agents, placed in 10 random cells
MyAgent.create_agents(model, 10, self.space.all_cells.choices_random_cells(k=10))

[github-actions]

Performance benchmarks:

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

[EwoutH]

Awesome, thanks for working on this!

Personally, since they are so similar, I would go for a single method (i.e. random or draw) with a keyword argument (replace or allow_duplicates). Like:

# Without replacement (equivalent to previous sample_random_cells)
cells = collection.random(k=10, replace=False)

# With replacement (equivalent to previous choices_random_cells)
cells = collection.random(k=10, replace=True)

But curious what others find intuitive naming.

[quaquel]

I have been going back and forth on this and in the end decided to stick close to the naming in the random library. Note that this also makes it possible to fully shadow the arguments from both methods which is virtually impossible if we collapse it into a single method with a boolean.

[EwoutH]

That's a fair point. In that case I would maybe suggest random_sample and random_choice. Cells is implied, because it's a method of a CellCollection, and starting them both with random helps when looking through methods alphabetically (in IDEs, etc.)

[quaquel]

Cells is implied, because it's a method of a CellCollection

That is not true. We already have a select_random_agent method on cell collection.

[EwoutH]

I would say that’s because those are agents selected from a collection of cells. That warrant naming it that way.

In the AgentSet we also name methods select and do, not select_agents and do_agents.

[quaquel]

Ok, but than I prefer simply sample, choices, (and perhaps choice) which is fully consistent with the naming of these methods in random.

[EwoutH]

I think the random_ prefix adds some value here (it’s also random.sample, and a sample or choice can also be non-random) but I’m also good without the prefix.

[quaquel]

I'll make up my mind and finalize it tomorrow.

[tpike3]

This is awesome! I had some nitpicky comments that I am not tied to at all.

I am good with merging I think only issue is changing the model1 in the visualization tutorial

[Corvince]

Oh, just saw this PR after my comments in #2530 . It does indeed seems to work to use cells directly. Honestly, I am not sure of the benefit of these methods, since they are indeed just thin wrappers and don't involve any logic themselves. But, yes, it helps discoverability so I am also not against it.

[quaquel]

I had overlooked the cells property when I made this PR. I am now, like @Corvince, not sure whether we need to merge this. I prefer to keep the API simple. Accidentally, we still have select_random_agent which is also in a way redundant because you can do self.random.choice(cellcollection.agents).

@EwoutH what do you think?

[EwoutH]

I agree it's more Pythonic to use built-ins. Good catch @Corvince.

The fact that we didn't realize it for this long, does say something about the discoverability. I think we can cover it with good documentation, but definitely something we would want to highlight in example models and a future tutorial.

I'm also good with deprecating/removing select_random_agent.

[quaquel]

closing this as not needed as @Corvince pointed out.

Should we add a deprecation warning to select_random_agent as well?