Skip to content

MRG, ENH: Add warning about bad whitener conditioning#8805

Merged
larsoner merged 7 commits intomne-tools:mainfrom
larsoner:cond
Feb 1, 2021
Merged

MRG, ENH: Add warning about bad whitener conditioning#8805
larsoner merged 7 commits intomne-tools:mainfrom
larsoner:cond

Conversation

@larsoner
Copy link
Copy Markdown
Member

@larsoner larsoner commented Jan 28, 2021

Attempt to address https://mne.discourse.group/t/strange-source-activity-with-lcmv-beamformer-in-volume-source-space-empty-room-covariance/2549

@SophieHerbst @hoechenberger can you see if this warns when you try your beamforming script? If not can you send me an evoked file to use? This was my testing script since I didn't have one:

Details 
import mne
cov = mne.read_cov('data-cov.fif')
noise_cov = mne.read_cov('noise-cov.fif')
fwd = mne.read_forward_solution('FORWARD_ico-4_MEGonly_CEREB-fwd.fif')
info = fwd['info']
info.update(comps=[], projs=[])

filters = mne.beamformer.make_lcmv(
    info, fwd, cov, noise_cov=noise_cov, rank=dict(meg=73), verbose=True)

And it now produces for me a nice warning, which I line-wrap here:

Computing rank from covariance with rank={'meg': 73}
Computing rank from covariance with rank={'meg': 73}
Making LCMV beamformer with rank {'meg': 73}
Computing inverse operator with 306 channels.
    306 out of 306 channels remain after picking
Selected 306 channels
Whitening the forward solution.
Computing rank from covariance with rank={'meg': 73}
    Setting small MEG eigenvalues to zero (without PCA)
/home/larsoner/Desktop/COV/rep.py:8: RuntimeWarning: The passed rank['meg']=73 exceeds the rank
estimated from the noise covariance (72) leading an increase in condition number by a
factor of 5e+12, this might amplify noise during whitening by a factor of 2e+06
  filters = mne.beamformer.make_lcmv(
Creating the source covariance matrix
Adjusting source covariance matrix.
Computing beamformer filters for 39825 sources
Traceback (most recent call last):
  File "/home/larsoner/Desktop/COV/rep.py", line 8, in <module>
    filters = mne.beamformer.make_lcmv(
  File "<decorator-gen-360>", line 22, in make_lcmv
  File "/home/larsoner/python/mne-python/mne/beamformer/_lcmv.py", line 176, in make_lcmv
    W, max_power_ori = _compute_beamformer(
  File "/home/larsoner/python/mne-python/mne/beamformer/_compute_beamformer.py", line 240, in _compute_beamformer
    raise ValueError(
ValueError: Singular matrix detected when estimating spatial filters. Consider reducing the rank of the forward operator by using reduce_rank=True.

The error is also there for me on main.

hoechenberger
hoechenberger reviewed Jan 28, 2021
View reviewed changes
hoechenberger
hoechenberger reviewed Jan 28, 2021
View reviewed changes
@hoechenberger
Copy link
Copy Markdown
Member

hoechenberger commented Jan 28, 2021

@larsoner Thanks, I couldn't test it so far, but I was just thinking: should we maybe even raise by default? Because this is a serious issue that will typically blow up your entire analysis, right?

@larsoner
Copy link
Copy Markdown
Member Author

larsoner commented Jan 28, 2021

This is just a heuristic, AFAIK it's not guaranteed to work. There might be realistic cases that trigger the warning, so I don't want to raise an error.

@hoechenberger
Copy link
Copy Markdown
Member

hoechenberger commented Jan 28, 2021

@larsoner Ok, fair enough!

@SophieHerbst
Copy link
Copy Markdown
Contributor

SophieHerbst commented Jan 29, 2021 •
edited
Loading

Hi @larsoner,
I tested it and I get to see the warning.

Now, I am not sure I would have known what to do about it before our discussion.
How about adding your recommendation of passing the rank of the matrix used to whiten?
(– and also add that to the documentation?)
Also, I don't understand what you mean by "condition number".

[While testing this I realized that the interactive console in VS code had all warnings disabled per default, so I might have missed them - thankfully I found a workaround now with @hoechenberger's help.]

@larsoner
Copy link
Copy Markdown
Member Author

larsoner commented Jan 29, 2021

Reworded and added to rank arg docstring, see if that's better @SophieHerbst !

@SophieHerbst
Copy link
Copy Markdown
Contributor

SophieHerbst commented Jan 29, 2021

yep, it is clear now @larsoner

agramfort
agramfort reviewed Jan 29, 2021
View reviewed changes
@agramfort
Copy link
Copy Markdown
Member

agramfort commented Jan 29, 2021 via email

@larsoner
Copy link
Copy Markdown
Member Author

larsoner commented Feb 1, 2021

Failures are just #8819 so I'll go ahead and merge

@larsoner larsoner merged commit 7e867ce into mne-tools:main Feb 1, 2021
@larsoner larsoner deleted the cond branch February 1, 2021 13:38
larsoner added a commit to adam2392/mne-python that referenced this pull request Feb 1, 2021
@larsoner
Merge remote-tracking branch 'upstream/main' into fixpersyst
a029254 
* upstream/main:
  MRG, ENH: Add warning about bad whitener conditioning (mne-tools#8805)
  MRG, MAINT: Deprecated param and pytest-qt (mne-tools#8808)
  fix mne.viz.plot_topomap with some missing grad in a pair (mne-tools#8817)
  [MRG] Coregistration-GUI:  use *.mff as digitization source (mne-tools#8790)
  FIX: Path
  [MRG] ENH EGI MFF reader:  populate info['dig'] (mne-tools#8789)
  MRG: Improve Brain UX (mne-tools#8792)
  FIX missing Axes3D import in viz._3d._plot_mpl_stc (mne-tools#8811)
  Better error message if configured download folder doesn't exist (mne-tools#8809)
  MRG, ENH: Add support for other formats to browse_raw (mne-tools#8807)
  MRG, BUG: Allow depth > 1 (mne-tools#8804)
@hoechenberger
Copy link
Copy Markdown
Member

hoechenberger commented Feb 1, 2021

Thank you, @larsoner!!

@larsoner larsoner mentioned this pull request Aug 30, 2023
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@agramfort agramfort agramfort approved these changes

+1 more reviewer

@hoechenberger hoechenberger hoechenberger approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@larsoner @hoechenberger @SophieHerbst @agramfort