Move NoiseGeneratorRecording from core to generation#4520
Open
h-mayorquin wants to merge 3 commits intoSpikeInterface:mainfrom
Open
Move NoiseGeneratorRecording from core to generation#4520h-mayorquin wants to merge 3 commits intoSpikeInterface:mainfrom
NoiseGeneratorRecording from core to generation#4520h-mayorquin wants to merge 3 commits intoSpikeInterface:mainfrom
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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.