Dimensionality consistency in RASR with sklearn API & pyriemann

[lkorczowski]

Edit 2020-02-10

Following the re-opening of this issue, the 2D compatibility will be completely drop (see https://github.com/bertrandlalo/timeflux_rasr/issues/5#issuecomment-584352525) The refactoring required consists of:

• [x] save an the result of a dummy test with 2D matrix and default parameters
• [x] only manage input of shape (n_trials, n_samples, n_electrodes) and discard any other input
• [x] remove all code epoching the data
• [x] remove all parameters related to epoching, i.e. blocksize, window_overlap, window_len
• [x] change the pytest to manage epoching outside of RASR
• [ ] compare results of the dummy 2D test with the same values with epoching outside (should be exactly the same)
• [ ] documentation to HOW to epoch for fit and transform of RASR

This is extensive refactoring, the code will be much simple but we will loose few in-build functionalities of ASR() (discussed in https://github.com/bertrandlalo/timeflux_rasr/issues/5#issuecomment-584352525)

---

Here is a tricky discussion that may or may not be important: dimension consistency.

The problem being that all the operations are not commutative, the choice of the order of the dimension in the data variable is the key to have everything working. Unfortunately, sklearn, pyriemann and other package dimension organization aren't consistent. It may not be a problem if a dimensions are double checked carefully but it still add several steps for transpose, reshape, and permutation operations.

More precisely:

• sklearn use (n_trials, n_features) as inputs. Leaving not space for multidimensional organization, other dimensions should be given as input (e.g. n_electrodes and n_samples)
• pyriemann made the decision to use (n_trials, n_electrodes, n_samples) and while not being exatly consistent to sklearn, it is at least consistent with mne.Epochs()
• other tooboxes such as Matlab RASR are using (n_trials, n_samples, n_electrodes) architecture which require (theoretically) less transpose operation.
• our RASR implementation use for now the latter while still managing and checking other possible inputs (such as 2D data structure of shape (n_samples, n_electrodes)).
• Timeflux seems to be based on the latter too.

While it is not an urgent work to do, for the release, I believe that this issue should be handled correctly because rightnow it is not fully compatible with pyriemann and mne (we can't stack directly too pipelines).

[lkorczowski]

Input and decision are required for sprint2 @mesca @sylvchev @bertrandlalo

[mesca]

Timeflux manipulates datetime-indexed dataframes. Because sklearn expects array-like objects, we provide df.values, so I confirm that you can expect a 2D data structure of shape (n_samples, n_electrodes).

If really needed, transposition and/or dimensionality reduction should be done outside of your estimator. We could provide light-weight transformers that would be chained in a sklearn pipeline.

[lkorczowski]

OK, let's try to make RASR works in Timeflux and eventually we will make a decision after detecting issues when they arise.

[lkorczowski]

we decided to follow as much as possible the sklearn style (more exactly mne and pyriemann implementation) with 3D inputs. However I will keep the opportunity to input 2D matrix for now because it is not much work.

selected choice

• other tooboxes such as Matlab RASR are using (n_trials, n_samples, n_electrodes) architecture which require (theoretically) less transpose operation.

We know it is imperfect (it doesn't follow exatly sklearn API nor pyriemann).

Decision arguments:

• sklearn first dimension is observation. However in our case an observation is an epoch. Taking the risk to have (n_samples, n_electrodes) makes RASR estimator poorly behave in sklearn pipeline (computing the reconstruction matrix require an entire epoch and not one sample). Therefore (n_trials, n_samples, n_electrodes) works.
• timeflux dataframe is (n_samples, n_electrodes) so we will can just stack those dataframes in second and third dimension without transpose (pyriemann is (n_trials, n_electrodes, n_samples), so RASR will require an operation in the pipeline to permute second and third dimension if needed).

NOTE: Decision is not yet completely fixed because of the last of clear consensus in the timeflux team.

[mesca]

The last time I checked, I had an error with 2D data. So either we fix this, or we completely drop 2D support. I currently use something like:

        steps:
          - module: timeflux.estimators.transformers.shape
            class: Expand
          - module: timeflux_rasr.estimation
            class: RASR
            args:
              srate: 250
          - module: timeflux.estimators.transformers.shape
            class: Reduce

Expand transforms to 3D and Reduce to 2D. Using 2D arrays directly and bypassing the first and last step does not work.

[lkorczowski]

I'm OK to drop 2D data compatibility but it will require quite a bit of code refactoring in fit() and cleaning the related input parameters. Note that 2D data compatibility was specifically made to simplified the training in fit(), without it the user will have to prepare some epochs with the right length and overlap.

Pro:

• drastically decrease the complexity of the RASR class (several parameters will be removed)
• less operations to do in RASR class
• more sklearn-friendly

Con:

• the user need to know by him/her-self what is the optimal parameters for window_len, overlap, etc. and prepare the epochs accordingly
• We might loose some of the matlab (r)ASR functionalities. It will require some time for me to double check all of tehm (preprocessing data to compute the reconstruction matrix is one of the functionalities as it is applied before epoching but this function was not yet implemented anyway).

I can put that in sprint3 #15 but before doing that, I'll need the snapshot of the error, could you provide me that?