ica.plot_properties ignores reject dict

[agramfort]

when you plot component properties you can specific the reject param used in fit.

reject should be set as attribute of ica object and be used with ica.plot_properties

cc @pierreablin @Okamille @jona-sassenhagen

[jona-sassenhagen]

@mmagnuski

[mmagnuski]

Ok, so we should add ica.reject (or something more verbose?) and then use this parameter automatically in plot_properties, right? Some questions then:

• Would you prefer that the data segments be rejected without any sign automatically or should the blanks be visible (nans in epoch's image)?
• Would it be useful to add a parameter to plot_properties that allows to ignore the ica.reject? In some cases you may want to investigate whether some epochs are worth rejecting (not included in further analyses) although they were not used in ica fitting.
• Or should it not be automatic and rather a reject kwarg should be added to plot_properties? It would be explicit to do ica.plot_properties(..., reject=True).
• Should something similar be done in the ica sources plotter? For example 'ignore' annotations showing which data segments were not used to fit ica.

[jona-sassenhagen]

Would you prefer that the data segments be rejected without any sign automatically or should the blanks be visible (nans in epoch's image)?

Yes, maybe reject could default to None or False and the image could then not show the dropped epochs at all? And reject=True means it pulls reject from the ICA object.

I'm not sure how well NaNs play with the smoothing.

Would it be useful to add a parameter to plot_properties that allows to ignore the ica.reject? In some cases you may want to investigate whether some epochs are worth rejecting (not included in further analyses) although they were not used in ica fitting.

ica.plot_properties(..., reject=False, ...)?

[mmagnuski]

Yes, maybe reject could default to None or False and the image could then not show the dropped epochs at all? And reject=True means it pulls reject from the ICA object.

with False it would not drop anything as I imagine, so the question about showing which data parts were rejected concerns reject=True.

I'm not sure how well NaNs play with the smoothing.

Right, that won't be good, the NaNs would spread like a disease. It would be possible to reject the segments, then smooth and then add rows with nans to the matrix. I think it might be useful to see what was rejected, but then some people would surely prefer not to, so that might require additional kwarg, so maybe we should think about it later?

ica.plot_properties(..., reject=False, ...)?

Yes, that's what I suggested in the next point (but didn't go back to the previous one to edit for clarity :) ).

[jasmainak]

If you want to have a parameter, I would suggest not changing the semantics. The param reject has always been a dictionary, so to me changing to be boolean in some parts of the code base is going to be confusing for many users. Why not simply use a rejection dictionary without saving it as an attribute in ica.reject? Is there a strong argument for believing that the rejection dictionary used during the fit is going to be valid for any other epochs for which you do ica.plot_properties ?

[mmagnuski]

@jasmainak Yes, that would be simpler. A middle-ground is to to have ica save reject dict and reject in .plot_properties() accepting a boolean or a dict.

[mmagnuski]

But I'd give my vote to just .plot_properties(..., reject=some_dict).

[agramfort]

fully agree with @jasmainak a reject param is a dict or None if reject was used during fit it must be used by default in plot_poperties now we can decide if we mark bad epochs in red or anything else or just ignore them. +1 however to be able to pass a different reject in the plot_properties method

[mmagnuski]

if reject was used during fit it must be used by default in
plot_poperties

@agramfort This is the point I am not sure of: at least when the dropping of segments/epochs would be silent (nothing in the visual output suggesting that something was removed). That might be surprising, especially for users accustomed to how things used to work in mne. Using ica.plot_properties(..., reject=ica.reject) or ica.plot_properties(..., reject=True) would be more explicit.

[jona-sassenhagen]

My main point is, I'd consider it very dangerous if plot_properties displays activity as if it had no artefacts. I.e., if you end up dropping all high-variance/high-amplitude epochs, the EOG ICs will look flat. So you will misidentify them.

I.e., I am very skeptical of dropping by default.

[mmagnuski]

good point, I agree with Jona.

[agramfort]

you could make the dots in the variance plot red and write the percentage of epochs that have been dropped? I'd like to see by default what the ICA solver had as input.

[mmagnuski]

Yes, I am less against making this the default when it is clearly visible that something was dropped.

[agramfort]

any volunteer to give it a try? cc @Okamille

…

[Okamille]

Yep, I'll work on it tommorow 😄

[Okamille]

Maybe we should consider using a reject_args instead of a reject, and include inside 'decim', 'reject', 'flat', 'tstep' and maybe 'picks' i.e channels to be used ?
That is what is used in the ICA object to perform the rejection, and they are not stored as members of the class afterward, so either we store these to perform the 'auto' rejection, or we ask the user to supply these parameters everytime.

[agramfort]

@Okamille keep is simple and stupid (KISS) and think about YAGNI (https://en.wikipedia.org/wiki/You_aren%27t_gonna_need_it). In other words let's add an option when it appears necessary as it is with the reject values for your experiment. I would address the comments you have now ASAP show how it addresses the issue to convince us and move on !

[Okamille]

What do you think about this way of conveying the info ?

[agramfort]

I would change the texts. ylabel should be:

Variance (AU)
and title
Dropped segments: 7.4%

I would have left the red dot at their original values as like this it's hard to know if the reject threshold is conservative or not. @dengemann or @jona-sassenhagen or @mmagnuski any option?

[mmagnuski]

I agree, having red dots at zero doesn't tell me much - it may almost suggest that these epochs/segments were flat.
Also - if we are showing rejected segments with red dots in variance plot I would also mark them in the epochs/segments image. The best option to do it is in my opinion:

• first calculate the image as previously
• then add rows filled with np.nan where removed epochs were originally

so it shouldn't be much of a problem, because you perform similar inserting for scatter. In case you you wanted to plot image nan elements in a different color than the default here is a nice example.

[Okamille]

So what do you think of this proposal ? Rejected epochs in image are white by default, I can use your tip @mmagnuski if you want another color :)

[jona-sassenhagen]

Is the spectrum calculated with the inclusion of missing values?

[Okamille]

No, it is computed from the "clean" epochs

[mmagnuski]

All looks good. I have only one question:
the variance plot suggests that we have altogether more than 50 segments, and the last segment is rejected. This can't be seen in the image plot: it suggests that there are not more than 50 segments and only 3 are removed (4 in variance plot).

[Okamille]

Oh yes, it's weird, I may have made some mistakes when adding the NaNs, I'll look into it

[jasmainak]

I would also suggest having consistent color between the subplots (red?) for bad epochs. Maybe work with facecolor and edgecolor to make it work?