Temporally correlated noise in simulations

Dear authors of core/generate.py's NoiseGeneratorRecording (seems to be @samuelgarcia @JoeZiminski @h-mayorquin),

I'm interested in extending this noise simulation to include temporal correlations, and I wanted to ask some questions about how best to do this without excessive rocking of boats. I'll open a PR to close this issue shortly with an implementation when the path is clear, assuming this feature is acceptable?

I already have an implementation for introducing temporal correlations (https://github.com/cwindolf/dartsort/blob/main/src/dartsort/evaluate/noise_recording_tools.py#L162, which basically just calls https://github.com/cwindolf/dartsort/blob/main/src/dartsort/util/noise_util.py#L213). This works by multiplying the data spatially by the square root of the covariance (inverse of whitener) and then doing some stuff in frequency domain to cause the data's power spectral density to match one that I computed from real data. Currently this is implemented as a BasePreprocessor subclass called UnwhitenPreprocessor which is applied over a WhiteNoiseRecording, so that correlations are introduced kind of the opposite of how the whitening step works in SpikeInterface (but also doing temporal stuff).

My questions:

• The current NoiseGeneratorRecording handles everything internally, rather than using a preprocessing chain like I am using. It seems to make sense to match this style, so that I would add more if/else cases to the NoiseGeneratorRecordingSegement.get_traces(). This will include adding some temporal margin if needed for the temporal part.
  • ❓ Does that sound like the right plan, or should I stick to the Preprocessor-based method I currently use?
• As part of my implementation, I would end up programming the logic for spatial correlation by multiplying by the covariance square root, rather than letting scipy's multivariate_normal() do its Cholesky stuff. It would be possible to unify the logic, so that the current spatial correlation would be reimplemented as white_noise @ spatial_cov_sqrt in all cases. But, this could change the output numerically and that may bother people.
  • ❓ Should I unify the spatial correlation into one logic which is the same whether or not there is also temporal correlation happening, or should I keep the current logic intact if there is only spatial covariance and handle the spatiotemporal stuff completely separately?

Those are my 2 questions. Let me know if you have any other concerns and thanks!

Start with core/generate.py, focusing on NoiseGeneratorRecording and NoiseGeneratorRecordingSegment.get_traces(), then compare the referenced dartsort noise implementation. Resolve with maintainers whether temporal correlation belongs in the existing generator or a preprocessing chain, and whether spatial covariance behavior should remain unchanged. Done means an agreed implementation path for temporal correlations.