Performance benchmarks:

Performance benchmarks:

I'm concerned we're accumulating some API bloat.

This PR adds .record() to DataSet and add_dataset() to BaseDataRecorder, resulting in this API:

self.recorder = DataRecorder(self)
self.data_registry.track_agents(self.agents, "agent_data", "wealth").record(self.recorder)
self.data_registry.track_model(self, "model_data", "gini").record(
    self.recorder, 
    configuration=DatasetConfig(start_time=4, interval=2)
)

In our recent discussion I thought we were moving towards an API like this:

# No explicit recorder construction - handled internally by model.data
self.data.track_agents(Wolf, "wolf_energy", "energy").record()
self.data.track_model(self, "gini", "gini").record(Schedule(interval=5, start=100))

With as main difference that .record() doesn't take a recorder argument and is internally managed).

With this PR, users now need to understand:

1. DataRecorder - construct it explicitly
2. DataSet.record(recorder, configuration) - new method that takes the recorder
3. BaseDataRecorder.add_dataset(dataset, configuration) - new public method
4. Configuration can be passed to both DataRecorder.__init__() and .record()

Do you see options to decomplicate both the mental model for users and the API? I think sensible defaults also would help to go a long way.

With as main difference that .record() doesn't take a recorder argument and is internally managed.

Users might use different recorders depending on the backend. So, the dataset needs to know the recorder that is to be used.

Also, at the moment, we don't have a default recorder field on the model, nor a configuration via __init__ to set it up. So, datasets cannot rely on any field on the model, nor do they, in general, have a reference to the model, even if a default recorder field did exist.

I agree about the end goal of having a clean API. This PR is just a small step to getting there. And I would argue it already simplifies it quite a bit because there is no longer the need to pass a complex configuration dict to the recorder. Instead, we tie the configuration directly to the dataset via the new record() method.

A next step, in my view, is to see if we can simplify DatasetConfig, potentially via Schedule, or at least by using the same keywords where appropriate.

Thanks for the context, sounds good.

A next step, in my view, is to see if we can simplify DatasetConfig, potentially via Schedule, or at least by using the same keywords where appropriate.

We might create a DataSchedule subclass. User can than:

• Pass nothing for the default (collecting ever 1 time)
• Pass a Schedule for simple functionality
• Pass a DataSchedule for more advanced functionality

It's a bit different. The two share start, end, and interval. But for data recording, interval only takes a number, but not a callable. In addition to these, DataSetConfig takes window_size, while Schedule takes count. So inheritance does not seem the best solution here.

Basically Schedule is a data storage object. It can have some fields that are useful/valid for recurring event scheduling, some that are useful for data recording, and some that are useful for both.

Basically Schedule is a data storage object. It can have some fields that are useful/valid for recurring event scheduling, some that are useful for data recording, and some that are useful for both.

Which is why a basic protocol with start, end, and interval might make sense, but a full hierarchy of classes is probably overkill.