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.

Sign up for GitHub

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

Remove Savitzky-Golay filter in suppression detection #102

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

Remove Savitzky-Golay filter in suppression detection #102

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
c79c437
remove savgol filter in suppression detection
May 19, 2021
53b334e
remove savgol filter
vloison May 19, 2021
8d0d89b
change threshold comparison
vloison May 19, 2021
044349a
add function test_artifact_in_suppression
May 19, 2021
48930d7
Merge branch 'envelop' of https://github.com/mattbit/neurokit into en…
May 19, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 2 additions & 3 deletions neurokit/analysis/suppressions.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,10 +64,9 @@ def _detect_suppressions(recording: Recording,

if threshold is None:
threshold = _find_threshold(rec.data.loc[:, channels])
envelope = rec.data.loc[:, channels].abs().values.max(axis=1)
envelope = savgol_filter(envelope, 3, 1)
envelope = np.nanmax(rec.data.loc[:, channels].abs().values, axis=1)
with np.errstate(invalid='ignore'):
ies_mask = envelope < threshold
ies_mask = envelope <= threshold
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we need this change?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because otherwise something occurs when, during the suppression, there is a single sample with value threshold, placed at a time t < min_duration after the beginning of the suppressions. In this case, the interval before the problematic sample is not considered as a suppression. This is what made the tests fail on the first commit. An AssertionError was raised when running test_detect_suppressions(3). Because the sample 532 was in the situation I mentionned before.
git pb

min_length = math.floor(min_duration * rec.frequency)
dilate_len = math.floor((min_duration + min_gap) * rec.frequency)
gap_len = math.floor(min_gap * rec.frequency)
Expand Down
20 changes: 20 additions & 0 deletions tests/analysis/test_suppressions.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -165,3 +165,23 @@ def test_artifacts():
analyzer = SuppressionAnalyzer(rec)
detections = analyzer.detect_ies(threshold=5.5)
assert len(detections) == 0


def test_artifact_in_suppression():
rec = simulated_eeg_recording(channels=['EEG_1'], duration=10,
frequency=100)
rec = rec.filter(1)
# Add suppression
rec.data.iloc[200:600] /= rec.data.iloc[200:600].values.max() / 5

# Add artifact
artifacts = EventSeries(name='artifacts')
artifacts.add(3.2, 4.8, code='test')
rec.es.add(artifacts)
analyzer = SuppressionAnalyzer(rec)
detections = analyzer.detect_ies(threshold=5.5)
assert len(detections) == 2
# The suppressions must not overlap with the artifact
sec = pd.Timedelta('1s')
assert detections.loc[0].end < 3.2 * sec
assert detections.loc[1].end > 4.8 * sec
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

probably start

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh yes, sorry about that, I'll change it