Code Style and Naming about braindecode HOT 19 CLOSED

from braindecode.

based on all of this how about:

• Trial
• (Compute) Window -> we use compute_window at some public function calls to give a strong hint at the beginning, internally can use shorter "window"
• Crop, stays as crop to stay consistent with paper, mostly used in internal code

from braindecode.

my only suggestion would be to use n_xxx to denote cardinalities (n_samples, n_features, n_epochs etc.)

from braindecode.

I created a wiki page to list these naming conventions: https://github.com/braindecode/braindecode/wiki/Variable-naming-conventions

I would vote for i_xxx instead of idx_xxx, and to use n_xxx as suggested by @agramfort.

from braindecode.

In the same pass as code style, we can also do naming stuff.
There we have to decide about supercrop-language, if we want to keep it and where etc.

from braindecode.

Another Point:

• Expression -> Lambda?

from braindecode.

from braindecode.

Working on #65 I've thought a bit more about naming conventions for supercrops vs. windows.

So we currently have the following concepts:

• Trial
• Supercrop
• Crop

Here are some thoughts:

• "Trials" are specific to recordings with some kind of paradigm or stimulus presentation (e.g., a BCI experiment). Sleep recordings, or clinical recordings (e.g., TUH) don't usually have trials, but instead have annotated regions, such as sleep stages, or arousal/seizure/etc. annotations. To avoid confusion (e.g., a user might wonder what a trial means in the context of sleep data?), I think what is currently called "trials" could instead be renamed "annotations". It also maps nicely to the concept of annotations (and annotations objects) in MNE.
• I think we agree that "supercrops" and "windows" mean the same thing (and both are used so far in the code). I would personally pick "windows" over "supercrops", as it's a more general term that anyone with basic knowledge of signal processing concepts should understand. I see "supercrops" as more of an implementation-specific term. Also, I think we agree that the other alternative, "epochs", is to be avoided given its meaning in DL.
• As for "crops", I believe the term currently only appears in samplers and models that explicitly require this concept. It makes sense to keep the work "crops" in this case.

Let me know what you guys think @robintibor @gemeinl @sbbrandt . When we've decided on a convention to follow I can open a PR to standardize functions and variable names.

from braindecode.

I think one issues is we have multiple things which refer to timewindows and differ in the meaning of the timewindows:

• Trials, as @hubertjb said, tied to a specific stimulus presentation/task
• Supercrops, tied to deep learning training loop
• Crops, tied to receptive field of deep network

How about we make this more explicit like:
Trial -> TrialWindow
Supercrop -> CropsWindow (or TrainingWindow? Maybe much more explicit? PredictWindow?)

As for "crops", I believe the term currently only appears in samplers and models that explicitly require this concept. It makes sense to keep the work "crops" in this case.

Yes, agreed, or at least we don't need to have very visible names for it atm

from braindecode.

I was actually thinking we should use terminology that is general enough to apply to the main use cases of the package. As long as we explicitly define these terms in the documentation, we should be good.

How about we make this more explicit like:
Trial -> TrialWindow

But then we would still have the word "trial" in there. I'm thinking specifically about this function: https://github.com/braindecode/braindecode/blob/master/braindecode/datautil/windowers.py#L18
Should we keep the word "trial"? I think "segment" as @agramfort suggested would apply to sleep, seizure annotations, etc. as well as BCI trials.

Supercrop -> CropsWindow (or TrainingWindow? Maybe much more explicit? PredictWindow?)

Why not just "windows"?

I suggest we include a little drawing in the documentation, along with some defintions. For example, I was thinking we could use the following terminology:

• Segment: A chunk of a recording that has been annotated, e.g., a trial in a stimulus-driven paradigm such as a BCI experiment, a sleep stage annotation, a seizure event, etc.
• Window: A group of consecutive EEG samples extracted for further analysis, typically between 0.5 and 30 s, and that are used as input to a neural network. Windows are sometimes called "supercrops" in the deep learning literature [refs] or "epochs" in the EEG processing literature [refs].
• Crop: A subdivision of a window, used in convolutional architectures to improve computational efficiency at training time. [refs]

We initially tried to define everything in the DL-EEG review, although some changes are required now:

from braindecode.

So my view atm:

• Trial
• ComputeWindow
• ReceptiveField (possibly only needed in very few places)

Then we use the established terms "trial" and "receptivefield", and have a new word for the computewindow, also signaling this should be chosen based on computational aspects.
We could also use something like ComputeCrop or whatever instead if we still want to take about "Cropped Decoding". That might be a good idae as well. Or we just call it Windowed Decoding or something?

from braindecode.

Thinking about it if we use ComputeCrop and this is only time we use crop, inside the code we could shorten to crop?

from braindecode.

@robintibor what do you mean by "computational aspects", can you explain better? What I understand is that compute window is a single input to the network? I wonder if it change interpretation of the variable name?

from braindecode.

yes it is a single input to the network. the larger you make this input, the more predictions you will get -> the more are computed in parallel, saving you computational time, and needing a bit more memory.

from braindecode.

I feel like windows are more important for problem definition than for computations, what do you think? From my perspective, this is how we define the EEG signal fragment for which we compute predictions and overall loss (we have predictions for all receptive_fields and average them to obtain final window prediction)? I would opt for "window" without the prefix, as atm I don't see a potential profit from adding "compute".
For the trial and receptive fields I think, they are quite clear. Trial was already introduced and it is easy to understand. Receptive field name shows that it is quite internal (the name is not so general and strictly connected to cropped decoding). Unfortunately, there is no connection in naming "cropped decoding" <-> "receptive field", so maybe we will need to change it somehow :/

from braindecode.

Unfortunately, "we have predictions for all receptive_fields and average them to obtain final window prediction" is not correct in general. For example, one use case that we had before and should continue to support is regressing a time series of a target. E.g. you have an EEG signal and a corresponding timeseries of a changing target, like position of a steering wheel the EEG user is controlling. In that case you only create windows to make computations feasible (can't input entire recording at once into the network), but the size of that window has no semantic meaning whatsoever, it is a purely computational choice (since you need different predictions at each timepoint, and larger windows just allow parallelization as discussed before). And actually for the case of trials with a single label where you cut compute windows inside, one can still view it equivalently in one of our main settings. Like you can also do it by never averaging the predictions, rather evaluating the loss for each prediction and then averaging the losses. this yields identical gradients in some settings (log softmax outputs / cross entropy loss).

from braindecode.

See https://onlinelibrary.wiley.com/doi/full/10.1002/hbm.23730 -> ConvNet Training-> Cropped Training for original motivation

The cropped training strategy uses crops, that is, sliding input windows within the trial, which leads to many more training examples for the network than the trial‐wise training strategy. [...] To reduce the computational load from the increased training set size, we decoded a group of neighboring crops together and reused intermediate convolution outputs. This idea has been used in the same way to speed up ConvNets that make predictions for each pixel in an image [Giusti et al., 2013; Nasse et al., 2009; Sermanet et al., 2014; Shelhamer et al., 2016]. In a nutshell, this method works by providing the ConvNet with an input that contains several crops and computing the predictions for all crops in a single forward pass (see Fig. 4 for an explanation). This cropped training method leads to a new hyperparameter: the number of crops that are processed at the same time. The larger this number of crops, the larger the speedup one can get (upper bounded by the size of one crop, see Giusti et al. [2013] for a more detailed speedup analysis on images), at the cost of increased memory consumption. A larger number of crops that are processed at the same time during training also implies parameter updates from gradients computed on a larger number of crops from the same trial during mini‐batch stochastic gradient descent, with the risk of less stable training.

from braindecode.

So we could still go through some namings this sprint like the n_ logic for cardinalities etc... if we want, but in a backward-compatible way via deprecation. Feel unsure about whether better to change or just keep as is... @hubertjb @sliwy @agramfort @gemeinl

from braindecode.

See #28 also

from braindecode.