search

aces / Loris

LORIS is a web-accessible database solution for longitudinal multi-site studies.
GNU General Public License v3.0
143 stars 172 forks source link

[electrophysiology_browser] Effect of filters between chunks #8717

Open jeffersoncasimir opened 1 year ago

jeffersoncasimir commented 1 year ago

Describe the bug In between chunks, there appears to be a gap, only visible when zoomed in. When applying filters, the gap becomes very visible, as the signals appear to diverge from the gap in a noticeable way. These filters should also be validated to determine if they are appropriate for more than one specific set of recording parameters.

Screenshot 2023-05-30 at 2 57 16 PM Screenshot 2023-05-30 at 2 57 31 PM

christinerogers commented 1 year ago

@jeffersoncasimir to take an hour before Friday 11 am and see if the solution is easy, consult with @laemtl if needed.

jeffersoncasimir commented 1 year ago

The cause for the gap is identified and is due to an off-by-one error.

This line should be inclusively producing all equally distributed numbers within the interval, but never equals interval[1] since i / values.length is never 1.

The unexpected filter behaviour between chunks is caused by filters being applied to chunks discretely as opposed to interpreting all visible points as a continuous signal.

christinerogers commented 1 year ago

est 2-4 weeks to refactor the filter application ->

could interpolation solve it ?

LF and JC to touch base

christinerogers commented 1 year ago

is this is beginner friendly issue @jeffersoncasimir ?

jeffersoncasimir commented 1 year ago

It is not

christinerogers commented 1 year ago

These filters should also be validated to determine if they are appropriate for more than one specific set of recording parameters. This is because they were hard-coded in the original Open MNI Atlas (for that recording, w values from N.von Ellenrieder) and then brought into LORIS. Not relevant to all recordings.

christinerogers commented 1 year ago

un/related: #8892 will be in the 25.1 release (ideally), but not too related to this issue which will persist

christinerogers commented 1 year ago

break this down, seems we have the following options (consult w LF and bring possible plans to the Loris-EEG meeting when ready)

  1. remove all filters
  2. @jefferson what's a small/medium scope solution? e.g. limit filter options, pre-cache...
  3. solve with dynamic calculations across chunk boundaries in the browser - what's the perfomance
jeffersoncasimir commented 11 months ago

A solution has been implemented, with a PR coming soon, which performs the signal filter on all visible chunks, instead of chunks individually. This is achieved by concatenating them all into one chunk/signal and works similarly to how DC offset was implemented.

This has a performance cost by introducing technically redundant memoizations but there is a lot of room for improvement on that front, which is due for a revisit.