Change phy exporter to not save template_ind in the case of dense waveform_extractor
#2148
Conversation
This file contains 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
alejoe91
reviewed
Nov 2, 2023
| if waveform_extractor.is_sparse(): | ||
| used_sparsity = waveform_extractor.sparsity | ||
| assert sparsity is None | ||
| if sparsity is not None: | ||
| warnings.warn("If the waveform_extractor is sparse the 'sparsity' argument is ignored") |
alejoe91
approved these changes
Nov 2, 2023
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.
Fixes #2023. Just trying to do a cleanup of the issue tracker for 0.99.0. Definitely could go later though.
2 things in this PR
As discussed in Dense vs Sparse template file saving for
export_to_phy#2023, there is no point in saving thetemplate_ind.npyif the data is dense since it is suppose to be the sparsity for phy. So I just set a boolean to not savetemplate_ind.npyif the waveform extractor is dense. Phy will use the lack oftemplate_ind.npyto treat it as a dense dataset so Phy's behavior isn't affected (might even be improved since it will treat the data as dense from the get go).with the new default of sparse=True (for
extract_waveforms()), I'm a little worried that users might not realize their input waveform_extractor is automatically sparse and they might try to layer a sparsity on top. In this case rather than stop the program with an assert I changed it to a warning, so that people know the waveform_extractor is already sparse. But if you want the assert instead that can be reverted.