-
-
Notifications
You must be signed in to change notification settings - Fork 1.5k
BUG: Fix bug with MF LCMV rank #11664
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
25ea762
9a14e80
1ae6fa3
fdb1d98
d5ce6d3
1568c6e
0ee6bdd
9efd7ab
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,3 +1,4 @@ | ||
| from contextlib import nullcontext | ||
| from copy import deepcopy | ||
| from inspect import signature | ||
|
|
||
|
|
@@ -1072,9 +1073,9 @@ def test_depth_does_not_matter(bias_params_free, weight_norm, pick_ori): | |
| assert_allclose(d1, d2, atol=atol) | ||
|
|
||
|
|
||
| @testing.requires_testing_data | ||
| def test_lcmv_maxfiltered(): | ||
| """Test LCMV on maxfiltered data.""" | ||
| @pytest.fixture(scope="session") | ||
| def mf_data(): | ||
| """Produce Maxwell filtered data for beamforming.""" | ||
| raw = mne.io.read_raw_fif(fname_raw).fix_mag_coil_types() | ||
| raw_sss = mne.preprocessing.maxwell_filter(raw) | ||
| events = mne.find_events(raw_sss) | ||
|
|
@@ -1084,10 +1085,26 @@ def test_lcmv_maxfiltered(): | |
| epochs = mne.Epochs(raw_sss, events) | ||
| data_cov = mne.compute_covariance(epochs, tmin=0) | ||
| fwd = mne.read_forward_solution(fname_fwd) | ||
| return epochs, data_cov, fwd | ||
|
|
||
|
|
||
| @testing.requires_testing_data | ||
| @pytest.mark.parametrize("use_rank", ("info", "computed", "full", None)) | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. so here, IIUC the point is that "info" and "computed" give the same answer (71); previously "info" gave the wrong answer post-maxfilter (right?). And the warning when using
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
In this case it's because it gets the rank estimate wrong. It's a known problem, though, and not what we're trying to fix here. Here we're just trying to fix that |
||
| def test_lcmv_maxfiltered(mf_data, use_rank): | ||
| """Test LCMV on maxfiltered data.""" | ||
| epochs, data_cov, fwd = mf_data | ||
| rank = compute_rank(data_cov, info=epochs.info) | ||
| assert rank == {"mag": 71} | ||
| for use_rank in ("info", rank, "full", None): | ||
| make_lcmv(epochs.info, fwd, data_cov, rank=use_rank) | ||
| ctx = nullcontext() | ||
| if use_rank == "computed": | ||
| use_rank = rank | ||
| elif use_rank is None: | ||
| ctx = pytest.warns(RuntimeWarning, match="rank as it exceeds") | ||
| with catch_logging() as log, ctx: | ||
| make_lcmv(epochs.info, fwd, data_cov, rank=use_rank, verbose=True) | ||
| log = log.getvalue() | ||
| n = 102 if use_rank == "full" else 71 | ||
| assert f"Making LCMV beamformer with rank {{'mag': {n}}}" in log | ||
|
|
||
|
|
||
| # To reduce test time, only test combinations that should matter rather than | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🙄