Repository navigation
Construction and state in extractors: a framework for serialization and saving #4939
Description
I have written this after our discussion in our maintainers meeting today as I have been thinking about this and I want to write a big framework here before commenting with what we discussed today on Graham's issue #4928.
Several bugs we fixed or discussed recently come from the same place: we don't have clear boundaries between the construction of an extractor and the different kinds of state it holds, so each serialization or saving path decides on its own what to carry. Some examples:
- #4928: the noise levels are cached as a property, so they are returned without checking the parameters that produced them.
- #4921:
select_periodsdropped the properties, annotations, registered recording and start times of a sorting. - #4883: the Open Ephys reader set its timestamps with
set_times, and after #4671 that got them dumped to the dict as if a user had set them. set_timesandshift_timesdo nothing on a preprocessed recording or on a frame slice of a recording with a time vector.
In addition, I have a few more timing bugs of this kind that I haven't filed yet as I do not want someone to fix them with AI and then I will get caught up with reviewing them (I would like to merge #4931 as soon as possible).
The purpose of this issue is to establish a framework so we can agree on what kinds of state we have and what each of them requires, and come back to it as we solve these problems. An extractor holds four types of content:
| Type | Description | Examples |
|---|---|---|
| Construction | The arguments of the constructor. | read_spikeglx(folder_path, stream_id="imec0.ap") |
| Format data | Data the reader sets from the format during __init__. Rebuilding from the construction sets it again. |
the gains, the channel locations, inter_sample_shift, the probe, the native timestamps |
| User state | State the user sets after construction. Only the user knows it. | recording.set_property("brain_area", ...), set_times(), a probe attached by hand, annotations |
| Computed state | Values computed from the traces. They can be rebuilt but it costs time and sometimes involves randomness. | the noise levels, W and M of whiten |
Right now format data, user state and computed state go through the same setters, so the extractor can't tell them apart. #4883 is that case for the times, and the metadata dict discussed in #4928 would record the origin for properties.
The most important point about state in SpikeInterface is that which types are required depends on the consumer and not on the extractor:
| Consumer | Construction | Format data | User state | Computed state |
|---|---|---|---|---|
Pickling to a worker or deepcopy |
carry | rebuilt | carry* | carry |
| Saving to binary, zarr or another format | carry | carry | carry | carry |
Provenance (provenance.json) |
carry | drop | drop | drop |
Preprocessing (bandpass_filter(recording)) |
delegate | delegate until set* | drop | |
recording.frame_slice(start_frame, end_frame) |
delegate | delegate until set* | delegate until set, if still valid | |
recording.select_channels(channel_ids) |
delegate | delegate until set | delegate until set |
Delegate means the derived recording reads its parent's value each time instead of storing a copy, unless its own __init__ sets a different one, as scale_to_uV does with the gains. Delegate until set means the derived recording reads its parent's value until it sets its own, for example with set_times, and from then on it keeps its own.
* This is what I think the library should do. Right now it does something different, which I think is an error: pickling drops the properties and annotations set by the user, and set_times and shift_times do nothing on a preprocessed recording or on a frame slice of a recording with a time vector (shift_times throws an error there). A lot of the bugs that I mentioned are cases of this: state is mishandled by some consumer.
Also, note that this is a general categorization to get the point through. A specific multiprocessing job can need almost no state or only a very specific part of it. For example, the workers of get_noise_levels only read traces and need none of the user state but in most cases, because the multiprocessing chain is general, all the state should be propagated as such.
To illustrate how this can be helpful:
- #4928: the noise levels are computed state stored as a property like user state, so they travel without the parameters that produced them. With computed state in its own place together with its parameters,
get_noise_levelschecks them before returning the cached value, andcopy_metadatadrops all computed state by rule instead of through the hardcoded_skip_propertieslist. - #4883: the native timestamps of Open Ephys are format data, but the reader set them with
set_times, the setter for user state, so after #4671 they were dumped to the dict. With a setter for format data the reader would have used it and serialization would skip them by rule. The fix sets the time vector of the segment directly. - #4921:
select_periodsdropped the properties, annotations, registered recording and start times of a sorting as it overwrote the sorting before callingcopy_metadata. The framework doesn't prevent that kind of mistake, but the rule "a selection keeps the format data and the user state" becomes a generic test that every selection and preprocessing step should pass, and that test would have caught it.
I think that having a separate place for each type, or at least a record of where each value comes from, is the way forward. I wrote this so we can think these scenarios through as we solve those bugs and add more features to the library.
Does this breakdown make sense? Am I missing a type or a consumer?
@alejoe91 @samuelgarcia @grahamfindlay @JoeZiminski @chrishalcrow
Activity
grahamfindlay commentedon Nov 13, 2026
I think that, overall, this makes sense. The thing I would want to think the most deeply about is whether these are the right types.
For example (somewhat provoked by #4928), is there really a clean distinction between "format data" and "user state"?
Say a reader sets channel locations from a file: format data. Then a user calls
set_probewith a different probe. Do the same locations now become user state? Same deal forset_channel_gains.Also, does "computed state" lump together two different things?
WandMofwhiten), computed once and stored in the constructor arguments (so they behave as construction -- they have to, if provenance drops computed state, per the table).And for subtype # 2 of computed state, this is a difficulty: whether a stored measurement is still correct after an operation doesn't depend on the operation alone -- it also depends on how the measurement was originally computed. For example, noise levels are computed from random chunks drawn across the whole recording. After selecting channels or units, the noise levels are still correct (assuming/because each channel's noise is computed on its own). After
frame_slice, the noise levels are arguably no longer correct (the original chunks might be outside the slide, and the slice's noise might differ). So I think you would need each kind of value/measurement in this # 2 subtype to declare, for each operation, whether to keep it, adjust it, or drop it (declares nothing -> dropped).Directly provoked by #4928: Can we leave a way for a pro user to inject their own value (this time not using public API) where a computed value normally goes? I do this a lot.
With respect to consumers, how do we deal with combining several extractors into one? Like
aggregate_channelsandconcatenate_recordings? I don't think any of carry, drop, or delegate say what happens when the inputs disagree. Right now I think every function decides for itself (e.g. concatenation takes the first recording's metadata).Also, I think SortingAnalyzer is missing as a consumer.
But I think I agree with the overall diagnosis.