ENH: Variable duration epochs

[larsoner]

We have some paradigms coming in where epochs are of variable duration (e.g., sentences are used). The current fixed tmax is unsuitable; [EDIT: if I have a set of all_tmax values in my paradigm, then using in the Epochs constructor] tmax=min(all_tmaxs) will cut off otherwise useful data from some epochs, and tmax=max(all_tmaxs) will lead to TOO_SHORT at the end of MEG runs.

Potential solutions:

1. Allow a variable tmax. This is probably much too hard to deal with given how our IO and class is structured.
2. Add a too_short='omit' | 'nan' kwarg to the Epochs constructor. In other words, if there are parts of the epoch (probably after the baseline period, which should probably still be required to be available in the Raw instance if baseline is not None) that are missing from the end, then just put np.nan in for the unavailable values.
   • This way users can use tmax=max(all_tmaxs) and be okay.
   • In this case, employing methods using the Epochs object can be considered to be caveat emptor, and we can make this clear in the docstring.
   • Over time we can make some of our functions smarter with a _check_finite function, which we should probably do anyway, really.
   • We'll be killed on epochs.save() storage a bit this way, but it's easily by using .fif.gz as those chunks will effectively disappear.

It seems like the (hopefully) little bit of added complexity from (2) is worth the enhanced functionality we gain. Thoughts?

[kingjr]

I'm not sure I completely understand the problem, you want to support epochs objects with variable durations?

[christianbrodbeck]

I am probably not aware of all the things one might want to do with those epochs, but might a Raw object also be a suitable container? That should at least make saving easier.

[mmagnuski]

I prefer the second solution. Fieldtrip has support for epochs of variable length (and so each epoch has its separate time: size(data.trial{i}, 2) == length(data.time{i})) but I have never found it very useful - in most cases variable epoch length problems can be solved with (sometimes multiple) fixed-sized epoch containers + some additional info (held somewhere else).
too_short='omit' doesn't sound like a problem but adding nans has the potential of breaking a lot of stuff... If many Epoch methods were not be reliable to use with too_short='nan' I'm thinking users might be better off using lists or dicts of Epochs (with keys like '<3s', '<5s' for example) and then cropping and joining all the data to one Epoch when necessary or performing relevant calculations separately etc.

[larsoner]

you want to support epochs objects with variable durations?

Kind of. Solution 1 would do that properly, solution 2 would allow users with variable durations to function hopefully more easily.

I am probably not aware of all the things one might want to do with those epochs, but might a Raw object also be a suitable container? That should at least make saving easier.

I'm not sure what you mean by making saving easier, as epochs.save() will still work, you'd just get a bit of wasted space unless you use GZip with solution 2. Of course it's possible to hijack some other object to accomplish what I'm talking about -- even object ndarray plus info could get some functionality -- but it wouldn't have the same effect.

To put this into context, consider what a user needs to do to get all trials from their paradigm epoched, baseline corrected, decimated, and inverted. In (2) it would be something very simple like:

epochs = Epochs(..., baseline=(None, 0), decim=5, tmax=biggest_tmax, too_short='nan')
stcs = apply_inverse_epochs(epochs, inv, ...)

And this will do the right thing, i.e. the nan will propagate only into the time points of stcs for trials in epochs that have nan in them. Some other functions will work properly (save, load), but others might not, like maybe TF.

To get this sort of thing to work in master currently, you either need to make multiple Epochs objects (ugly, doesn't save nicely, etc.) or do a bunch of arithmetic juggling with a Raw object to do it, which is duplicating a bunch of functionality and non-trivial for users.

adding nans has the potential of breaking a lot of stuff

Yes it will break some things, but it will also not break other (critical) things like minimum-norm source localization -- thus the caveat emptor comment. But really it will work with a number of functions out of the box, such as apply_inverse_epochs.

... I'm thinking users might be better off using lists or dicts of Epochs (with keys like '<3s', '<5s' for example) and then cropping and joining all the data to one Epoch when necessary or performing relevant calculations separately etc.

With proposal (2) they can easily do the "cropping and joining" step of this proposal just with epochs.crop(0, min(all_tmaxs)), so I don't see why this proposal would be advantageous over (2). We can make it clear that to get all Epochs-related functions to work, such cropping could be necessary. This combined with some additions of a _check_finite call before functions (like TF and probably decoding) could go a long way.

[kingjr]

I'm -1 on supporting epochs with variable lengths.

• It is a Pandora box as we'll have to fix time frequency, psds, plotting,
  the definition of the times attributes, the covariance, Ica, the nave for
  sources, stats etc to have exception cases for for the nans.
• I would argue that variable lengths is an intrinsically bad design for an
  epochs container, as the container is supposed to handle repetitions that
  are meaningfully comparable with one another.
• If one wants to identify sources or spatial filters they can always use
  rERP.
• It is a rare use case. My intuition is that most protocols with variable
  lengths have a few possible lengths and can be stored as separate epochs.

Le 26 août 2016 07:47, "Eric Larson" notifications@github.com a écrit :

you want to support epochs objects with variable durations?

Kind of. Solution 1 would do that properly, solution 2 would allow users
with variable durations to function hopefully more easily.

I am probably not aware of all the things one might want to do with those
epochs, but might a Raw object also be a suitable container? That should at
least make saving easier.

I'm not sure what you mean by making saving easier, as epochs.save() will
still work, you'd just get a bit of wasted space unless you use GZip with
solution 2. Of course it's possible to hijack some other object to
accomplish what I'm talking about -- even object ndarray plus info could
get some functionality -- but it wouldn't have the same effect.

To put this into context, consider what a user needs to do to get all
trials from their paradigm epoched, baseline corrected, decimated, and
inverted. In (2) it would be something very simple like:

epochs = Epochs(..., baseline=(None, 0), decim=5, tmax=biggest_tmax, too_short='nan')
stcs = apply_inverse_epochs(epochs, inv, ...)

And this will do the right thing, i.e. the nan will propagate only into
the time points of stcs for trials in epochs that have nan in them. Some
other functions will work properly (save, load), but others might not, like
maybe TF.

To get this sort of thing to work in master currently, you either need to
make multiple Epochs objects (ugly, doesn't save nicely, etc.) or do a
bunch of arithmetic juggling with a Raw object to do it, which is
duplicating a bunch of functionality and non-trivial for users.

adding nans has the potential of breaking a lot of stuff

Yes it will break some things, but it will also not break other
(critical) things like minimum-norm source localization -- thus the caveat
emptor comment. But really it will work with a number of functions out
of the box, such as apply_inverse_epochs.

... I'm thinking users might be better off using lists or dicts of Epochs
(with keys like '<3s', '<5s' for example) and then cropping and joining all
the data to one Epoch when necessary or performing relevant calculations
separately etc.

With proposal (2) they can easily do the "cropping and joining" step of
this proposal just with epochs.crop(0, min(all_tmaxs)), so I don't see
why this proposal would be advantageous over (2). We can make it clear that
to get all Epochs-related functions to work, such cropping could be
necessary. This combined with some additions of a _check_finite call
before functions (like TF and probably decoding) could go a long way.

—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
#3533 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AEp7DLOYmKdOjOjRmdpNZ8GLdjReffPjks5qjtESgaJpZM4JtrNC
.

[jona-sassenhagen]

I agree with JR.

I use variable-length epochs stuff (sentences or short stories) a lot - actually, most of what I do is like that - so I would possibly benefit from this, but I think "Epochs" should indeed refer to comparable things, whereas variable-length entities are probably most appropriately captured by raw objects.

Maybe it would be useful to allow writing multiple epoch objects or raw objects to one file though.

[mmagnuski]

@kingjr I agree, but @Eric89GXL second proposition does not directly involve epochs with variable lengths but supports a workaround for people who need it.

[larsoner]

It is a Pandora box as we'll have to fix time frequency, psds, plotting, the definition of the times attributes, the covariance, Ica, the nave for sources, stats etc to have exception cases for for the nans.

Most of these do not apply to solution (2) -- inverses, many platting functions, the times attriubutes, etc. would not be affected. For other functions like TFR that will break with nans present, we can add a simple _check_finite function to them, which we should probably do in master regardless to be safe. So I don't see this as a big problem at the implementation or user end.

I would argue that variable lengths is an intrinsically bad design for an epochs container, as the container is supposed to handle repetitions that are meaningfully comparable with one another.

That is one use, yes, but is too narrow a view of the function of Epochs I think -- they also are the standard for how we parse Raw files based on events, put those through inverses, decimate, etc. And even with putting in nan, up through min(all_tmaxs) the epochs would be comparable in a reasonable way, fitting your definition up to a certain time, which the user can easily get with epochs.crop(0, min(all_tmaxs)).

If one wants to identify sources or spatial filters they can always use rERP.

I don't think we should force people to use non-standard analyses.

It is a rare use case.

This is at least the third time someone in my lab has wanted something like this (and we are one lab), so I disagree. I think there are probably people like us who have hit this problem before, and just lived with a few epochs being dropped due to TOO_SHORT, or otherwise hacked around it.

My intuition is that most protocols with variable lengths have a few possible lengths and can be stored as separate epochs.

This is a messy solution. Think about how a user would epoch 100 sentences, save them, load them, and invert them. My code snippet above does it in two lines, with one file saved to disk, one file loaded, etc. Your proposal would require a lot of extra work (and files, etc.) at the user end. And also think about cases where they do want to analyze the first min(all_tmaxs) segment of time together -- now they have to things like crop them all and concatenate (and make sure they've been handled equivalently the whole time) instead of just calling .crop(...).

[jona-sassenhagen]

Okay, that's good points. Hm.

[kingjr]

Isn't the original problem more related to the fact that the MEG runs were stopped too quickly (wrt the longest epochs) rather than a true support of multiple epoch lengths?

If so, ok for (2), although you could probably pad the Raw data with extra values rather than adding an extra construct param?

[larsoner]

Isn't the original problem more related to the fact that the MEG runs were stopped too quickly (wrt the longest epochs) rather than a true support of multiple epoch lengths?

I would word the true problem as, for paradigms with variable epoch durations, we don't have a good way to extract epochs for them. One way this manifests as the runs ending too early relative to the longest trial, yes.

If so, ok for (2), although you could probably pad the Raw data with extra values rather than adding an extra construct param?

Yes it's true there are work-arounds, but none of them are all that nice, including IMO this one. It needs (at least) four extra steps before actually calling Epochs(...): computation of additional necessary number of samples to add, concatenation, construction of a new RawArray, and subtraction of first_samp from events (as this gets set to 0 in RawArray). This will require more memory and adds failure points, especially with the treatment of first_samp.

[agramfort]

same first reaction @kingjr but I am eager to see a draft of solution 2 as I agree that it's not too intrusive for the most of us who don't need such a feature.