Skip to content
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

Question/Suggestion: Rethink / Adjust to suboptimal up-/downsampling in to_dense() #368

Closed
adswa opened this issue Jan 29, 2019 · 3 comments

Comments

@adswa
Copy link
Contributor

adswa commented Jan 29, 2019

We have been looking further into the issue of the extra column during resampling (#358, #365) and ended up with a bit of confusions regarding the implementation of to_dense().

duration = int(math.ceil(sampling_rate * self.get_duration()))
ts = np.zeros(duration, dtype=self.values.dtype)
onsets = np.round(self.onset * sampling_rate).astype(int)
durations = np.round(self.duration * sampling_rate).astype(int)
run_i, start, last_ind = 0, 0, 0
for i, val in enumerate(self.values.values):
if onsets[i] < last_ind:
start += self.run_info[run_i].duration * sampling_rate
run_i += 1
_onset = int(start + onsets[i])
_offset = int(_onset + durations[i])
ts[_onset:_offset] = val
last_ind = onsets[i]

It seems to be that during up or downsampling, the data is simply "broadcasted" instead of interpolated? Downsampling via this broadcasting and subsequent linear interpolation during 'proper' downsampling in resample() would then change the data even if nothing was done to it. @yarikoptic suggest that the default kind in resample should be 'nearest' if its broadcasting instead of linear interpolation here. Or instead to use linear interpolation to fill the values.

Are we thinking correct here?

@tyarkoni
Copy link
Collaborator

tyarkoni commented Jan 30, 2019

Not sure I follow... to_dense can only be called on a SparseRunVariable. Each event in a SparseRunVariable is represented strictly in terms of onset, duration, and amplitude. We're just taking that temporal information and filling a uniformly-sampled array with boxcars that have the appropriate onsets and offsets. There's no resampling happening. (I think maybe your confusion is that both sparse and dense variables store their values in .values. But the meaning is different, and since to_dense can't be called on a DenseRunVariable, you would never end up doing any actual resampling of timeseries data.)

@yarikoptic
Copy link
Collaborator

Heh, I think it is my bad reading of the code here. Sorry about the noise

@tyarkoni
Copy link
Collaborator

No worries—happy to tolerate a little noise given all the signal today. :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants