Move NoiseGeneratorRecording from core to generation#4520
Move NoiseGeneratorRecording from core to generation#4520h-mayorquin wants to merge 3 commits intoSpikeInterface:mainfrom
NoiseGeneratorRecording from core to generation#4520Conversation
|
Hi @h-mayorquin and @samuelgarcia , I'm interested in working on simulated data and would love to see this PR merged so that I can rework my own PRs. @h-mayorquin , from the discussion at #4522, it seems like you plan to remove the |
|
Maybe to keep things moving we could merge this PR as-is and handle the strategy stuff later? I can work on the changes related to the NoiseGeneratorRecording in a follow up PR. |
|
I think this is up to @samuelgarcia. Alessio is on vacations now. |
|
I think @alejoe91 is back @alejoe91 @samuelgarcia any blocker on this? |
| The sampling frequency of the recorder. | ||
| durations : list[float] | ||
| The durations of each segment in seconds. Note that the length of this list is the number of segments. | ||
| noise_levels : float | np.ndarray, default: 1.0 |
There was a problem hiding this comment.
I would actually keep noise levels here. It's useful to have a mock recording test where you can easily modify them. What do you think @h-mayorquin
I agree with rmeoving the cov_matrix!
There was a problem hiding this comment.
Can't think on a use case for that, do you have something specific on mind?
There was a problem hiding this comment.
As in, in the context of #4482 (comment) I don't see how that would fit on the scope of the mock.
Addresses the design discussion in #4482.
NoiseGeneratorRecordingmixed two purposes: a lightweight lazy recording for testing and a simulation tool with spatial correlations (cov_matrix) and per-channelnoise_levels. This PR introduces a minimalMockRecordingincore/generate.py(unit-variance white noise only, no extra parameters) and moves the fullNoiseGeneratorRecordingtogeneration/noise_tools.py, wheregenerate_noise()already lived.generate_recording()andgenerate_recording_by_size()now useMockRecording, whilegenerate_ground_truth_recording()lazy-importsNoiseGeneratorRecordingfrom generation. That way, the work on #4482 can proceed from here without interfering with the testing functionality. I will migrate the tests in a follow-up PR so this diff stays as clear as possible with minimal code churn.Backward compatibility is preserved via module-level
__getattr__in bothcore/__init__.pyandcore/generate.py. ImportingNoiseGeneratorRecordingfrom the old paths still works but emits aFutureWarningdirecting users to import fromspikeinterface.generationinstead, with removal targeted for 0.106.0. I also changedSilencedPeriodsRecordingto composeMockRecording+ScaleRecordinginstead of importingNoiseGeneratorRecordingfrom core, to comply with a design decision once discussed with @samuelgarcia about the way modules should depend on one another (preprocessing should not depend on generation). I am happy to import from generation directly if you think that is better.MockRecordingstill exposes thestrategyargument for now. I would like to remove it and keep onlytile_pregenerated, but I opened #4522 to discuss whether the simulation side needson_the_flyfor non-repeating noise. I will clean this up in a follow-up once that discussion settles.