Warn if event sample number is out of bound for raw when creating Epochs

[skjerns]

Description of the problem

When creating Epochs by providing an events array that contains sample numbers outside of the data, the respective epochs are dropped, but no warning is given that sample numbers are actually out of bounds.

This can happen e.g. if events is in another sampling frequency as raw or if events contains sample numbers before first_sample.

related: https://mne.discourse.group/t/dropped-epochs-when-preload-true/2503

Steps to reproduce

import numpy as np
import mne

# Generate random data
data = np.random.randn(5, 1000)

# Create MNE info structure
ch_names = [f'EEG {i+1}' for i in range(5)]  # Channel names
info = mne.create_info(ch_names=ch_names, sfreq=100)

# Create RawArray
raw = mne.io.RawArray(data, info, first_samp=150)


events = np.array([[50, 0, 1],  # before first_samp
                   [500, 0, 2],  # works
                   [1100, 0, 3]])  # outside of data

epochs = mne.Epochs(raw, events=events, preload=True)

Expected results

I would expect that MNE warns me that the events array contains sample numbers that are out of bounds, instead of marking them as bad epochs.

Actual results

Out of bounds epochs are dropped.

3 matching events found
Setting baseline interval to [-0.2, 0.0] s
Applying baseline correction (mode: mean)
0 projection items activated
Using data from preloaded Raw for 3 events and 71 original time points ...
2 bad epochs dropped

[scott-huberty]

I agree that it is worthwhile to warn users about an events array containing out-of-bound samples!

I'd argue that warn + drop those epochs is the right approach, as opposed to warn instead of dropping those epochs (as you suggest). (EDIT: If I'm understanding your expected results correctly).

This preserves current behavior (just makes users more aware of it), and prevents an epoch from being created where there is no data (as is the case for the last event in your events array).

[larsoner]

We could add a new on_event_bound="warn" | "raise" | "ignore" where "ignore" is essentially the old behavior and "warn" is the behavior proposed here.

We would have to make it clear that it only will emit a warning when the event sample in the events array is out of bounds (assuming that's what we indeed want). For example if I have equally spaced events starting at sample zero but tmin=-0.2, tmax=0.8 the first epoch will not exist / be dropped because to create that epoch you'd have to take from negative time in the raw instance. But that's less clearly an events-creation problem than having an event at sample 10000 when you only have 8000 samples (and first_samp=0).

[scott-huberty]

@larsoner do you think there will be downstream side-effects in the case of on_event_bound="warn"? E.g. in your example, what happens when you do epochs.apply_baseline given that the first epoch won't have any data from -.2-0 s?

[larsoner]

E.g. in your example, what happens when you do epochs.apply_baseline given that the first epoch won't have any data from -.2-0 s?

Good question, and not 100% sure. I'm inclined to say we shouldn't warn in this case, we should just warn in the case that the zeroth sample number is outside the bounds.

But as far as downstream side effects go emitting warnings is more of a dev problem than a user problem. Users are capable of ignoring such warnings easily :) For devs it will require some work to make it so that test suites that treat uncaught warnings as errors (like ours, mne-bids, mne-bids-pipeline, etc.) pass again, which will be annoying. But it seems worth it to help users not make mistakes.