-
Notifications
You must be signed in to change notification settings - Fork 124
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
ENH: Improve convolution performance for Sparse variables #411
Merged
+79
−16
Merged
Changes from all commits
Commits
Show all changes
18 commits
Select commit
Hold shift + click to select a range
29b9df5
Do not convert to Dense if variable is Sparse
adelavega ffdaa01
Set resample_frames to uniform 10hz if Sparse, return Dense
adelavega 857cd0f
get_duration on var
adelavega 40c5dec
Default to 1 Hz
adelavega 625d67d
Return events at 10Hz, enable adapative oversampling
adelavega b4881c8
Take min of duration, and do future division
adelavega 129a30b
Simplify adapative oversampling code
adelavega 44825e2
Update bids/analysis/transformations/compute.py
effigies 49547ea
Lint convolve function
adelavega 814ee3f
Merge branch 'fix/convolve' of github.com:bids-standard/pybids into f…
adelavega cd7821c
Use df onset and duration, not var
adelavega 258c26e
Add min_interval comment
adelavega 38f6cc4
Compute min_interval based on unique values
adelavega 46f638e
Add test_convolve, with mocking for testing oversampling factor
adelavega 0e18ff7
Mock Python 2
adelavega 24646c8
Double oversampling, (pre ceil rounding)
adelavega d3e50ee
Check that there are at least two events to compute min diff
adelavega 916c8a3
Use same formula as DenseRunVariable._build_entity_index to calculate…
adelavega File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
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
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm wondering if we should center the sampled frames within the timeseries... e.g., suppose we have a 2-second dense timeseries, and we want to downsample to 2 Hz. Currently we would sample at 0, 0.5, 1, and 1.5. Probably we should do 0.25, 0.75, 1.25, and 1.75. Let's not change anything here, because if we were going to do this, we'd need to do it throughout the codebase for consistency. Mostly just making a mental note.