Adding ability to get minimum and maximum peaks from evoked object
Describe the new feature or enhancement
Adds the ability to find minimum and maximum peaks not just negative and positive.
Describe your proposed implementation
Append a simple conditional to the _get_peak() function.
Usage:
evoked.get_peak(mode="max")
Describe possible alternatives
N/A
Additional context
No response
not sure I understand what you're seeking here:
evoked.get_peak("pos")returns the time of the most extreme positive valueevoked.get_peak("neg")returns the time of the most extreme negative valueevoked.get_peak("abs")returns the time of the most extreme value (positive or negative, whichever has largest absolute value)evoked.get_peak("max")returns ???evoked.get_peak("min")returns ???
Hey @drammock! I think the get_peak("pos") just considers positive values and get_peak("neg") just considers negative. That means that if you have only positive values there is no way to find the minimum of the positive values. See this forum post:
https://mne.discourse.group/t/can-get-peak-function-specify-the-maximum-or-minimum-value/2673
ok, I understand now.
The suggested work-around provided in that forum post gives the per-channel minima and is only 2 lines:
idx_min = np.argmin(evoked.data, axis=1) # min for each channel
t_signal_min = evoked.times[idx_min]
Going from that to the global min isn't much more:
ch_idx, time_idx = np.unravel_index(np.argmin(evk.data), shape=evk.data.shape)
ch_name = evk.ch_names[ch_idx]
min_time = evk.times[time_idx]
min_amp = evk.data.min() # or evk.data[ch_idx, time_idx]
If I had to guess, the method was written the way it is because it's designed to test a scientific assumption (something like "in this condition there ought to be a negative extremum around N milliseconds") and in a context like that it is arguably a good thing to fail if the input evoked is all positive. The use case of "which channel has the smallest positive value in some all-positive data" was (I guess?) not judged to be useful / frequent enough to need a convenience method.
So I guess my questions to you @withmywoessner are:
- is this a frequent analysis need?
- are you doing this in a context where the other stuff done by
get_peak(i.e., merging grad channels) is necessary? - is the equivalent numpy code above too complicated / too hard to discover / otherwise inadequate?
If we're going to alter how this method works, I'm a bit hesitant to have options both "pos" and "max" (or "neg" and "min"), they seem too similar and thus likely to cause confusion. I'd rather have something like evk.get_peak(..., mode="neg", on_sign_mismatch="info"|"warn"|"raise") so that users could disable the constraint that says "error if asked for neg and data are all pos". Also open to other API ideas.
- is this a frequent analysis need?
- are you doing this in a context where the other stuff done by
get_peak(i.e., merging grad channels) is necessary?- is the equivalent numpy code above too complicated / too hard to discover / otherwise inadequate?
- is the equivalent numpy code above too complicated / too hard to discover / otherwise inadequate?
If I wanted to the restrict the time-ranges considered to detect peaks of event related potentials. I think the numpy code would be not that complicated, but for someone like me who is not that adept in numpy may be a bit confusing. The code itself uses a mask to do this I think?
if tmin is None:
tmin = times[0]
if tmax is None:
tmax = times[-1]
if tmin < times.min() or tmax > times.max():
if tmin < times.min():
param_name = "tmin"
param_val = tmin
else:
param_name = "tmax"
param_val = tmax
raise ValueError(
f"{param_name} ({param_val}) is out of bounds. It must be "
f"between {times.min()} and {times.max()}"
)
elif tmin > tmax:
raise ValueError(f"tmin ({tmin}) must be <= tmax ({tmax})")
time_win = (times >= tmin) & (times <= tmax)
mask = np.ones_like(data).astype(bool)
mask[:, time_win] = False
I did not want to deal with this in my own script so I just ending up using pandas before making this change.
The evk.get_peak() is very convenient as it already allows you to restrict the time intervals:
get_peak(tmin=.500 tmax=.800, mode='min', return_amplitude=True)
I'd rather have something like evk.get_peak(..., mode="neg", on_sign_mismatch="info"|"warn"|"raise") so that users could disable the constraint that says "error if asked for neg and data are all pos". Also open to other API ideas.
That sounds good. @drammock
ok, so the other stuff done by get_peak() is needed. Would this API work / make sense for you? Seems cleaner than what I suggested before
evoked.get_peak(mode="neg", strict=False)
where new keyword arg strict determines whether to error out if mode="neg" and there are no negative values (the current behavior, would become the strict=True behavior), or instead to happily return the minimum value of the all-positive data (the strict=False behavior).