FIX: check weight_norm parameter

[massich]

Reference issue

Example: Fixes #5945

What does this implement/fix?

checks that weight_norm : 'unit-noise-gain' | 'nai' | None

Additional information

@jaaval now this snipped errors with a meaningful error

import mne
from mne.datasets import sample
from mne.beamformer import make_lcmv

data_path = sample.data_path()
raw_fname = data_path + '/MEG/sample/sample_audvis_raw.fif'
event_fname = data_path + '/MEG/sample/sample_audvis_raw-eve.fif'
fname_fwd = data_path + '/MEG/sample/sample_audvis-meg-eeg-oct-6-fwd.fif'
label_name = 'Aud-lh'
fname_label = data_path + '/MEG/sample/labels/%s.label' % label_name
subjects_dir = data_path + '/subjects'

event_id, tmin, tmax = 1, -0.2, 0.5

# Setup for reading the raw data
raw = mne.io.read_raw_fif(raw_fname, preload=True)
raw.info['bads'] = ['MEG 2443', 'EEG 053']  # 2 bads channels
events = mne.read_events(event_fname)

# Set up pick list: EEG + MEG - bad channels (modify to your needs)
picks = mne.pick_types(raw.info, meg=True, eeg=False, stim=True, eog=True,
                       exclude='bads')

# Pick the channels of interest
raw.pick_channels([raw.ch_names[pick] for pick in picks])
# Re-normalize our empty-room projectors, so they are fine after subselection
raw.info.normalize_proj()

# Read epochs
epochs = mne.Epochs(raw, events, event_id, tmin, tmax,
                    baseline=(None, 0), preload=True, proj=True,
                    reject=dict(grad=4000e-13, mag=4e-12, eog=150e-6))
evoked = epochs.average()

forward = mne.read_forward_solution(fname_fwd)
forward = mne.convert_forward_solution(forward, surf_ori=True)

# Compute regularized noise and data covariances
noise_cov = mne.compute_covariance(epochs, tmin=tmin, tmax=0, method='shrunk',
                                   rank=None)
data_cov = mne.compute_covariance(epochs, tmin=0.04, tmax=0.15,
                                  method='shrunk', rank=None)

filters = make_lcmv(evoked.info, forward, data_cov, reg=0.05,
                    noise_cov=noise_cov, pick_ori=None,
                    weight_norm='asdf', rank=None)

This is the result of runing the MWE

ValueError: Unrecognized weight normalization option in weight_norm, available choices are "nai", "unit-noise-gain" and None; got "asdf".

--
@wmvanvliet can you make sure that this is ok? The 'nai' option was added in
#5447. I've seen that tf_lcmw the 'nai' option is not there. And this PR now
makes 'nai' valid for _lcmv_source_power.

Maybe we can also use this PR to update the doc and make use of the doccer that
@larsoner put in place.

I also see this kind of checkings in many places, maybe we should factor it out and
have a prameter checker or something, that unifies the errors etc.

[codecov]

Codecov Report

Merging #5946 into master will decrease coverage by 1.81%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master    #5946      +/-   ##
==========================================
- Coverage   88.91%   87.09%   -1.82%     
==========================================
  Files         401      401              
  Lines       72825    72825              
  Branches    12109    12109              
==========================================
- Hits        64750    63426    -1324     
- Misses       5189     6561    +1372     
+ Partials     2886     2838      -48

[agramfort]

good to go or not?

[wmvanvliet]

I agree with adding the check, but this implementation is too verbose for my taste. Why not add at the appropriate place in _compute_beamformer the following, once:

if weight_norm not in ['nai', 'unit-noise-gain', None]:
    raise ValueError('Unrecognized weight normalization option, '
                     'available choices are "nai", "unit-noise-gain" and None; '
                     'got "%s".' % weight_norm)

[wmvanvliet]

My apologies, I hadn't read the PR carefully.

I also see this kind of checkings in many places, maybe we should factor it out and
have a prameter checker or something, that unifies the errors etc.

This is a good idea. This situation comes up frequently and may indeed warrant a convenience function. However, this is probably best left to a separate PR. In this PR, let's implement the check with as little code as necessary.

[agramfort]

@massich can you rebase this one and let's merge. thanks a lot

addressed in #5958

[massich]

@wmvanvliet merge if happy. Then you would have to remove these lines in #5958.

But I guess that what I was asking at the beginning still holds: do both methods support the 'nai' and 'unit-noise-gain'? If so, then this is fine. If not this is broken and the easiest is to add what you suggested (or wait for #5958 and add two different checks). The blame says that 'nai' was written by @wmvanvliet

[wmvanvliet]

No! _lcmv_source_power does not support weight norms other than 'unit-noise-gain'!

[wmvanvliet]

This is one for the beamformer agenda :)

[agramfort]

@wmvanvliet you can push directly to the PR as core dev of MNE-Python. I think it's the fastest way to move on. If you know what needs to be done it's faster to push the 2 lines fixes that going over 3 rounds of comments.

my 2c

[wmvanvliet]

Thanks, @massich!