Skip to content

FIX: Add on_few_samples parameter to core rank estimation #13350

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.

Sign up for GitHub

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

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
+174 −56
Open

FIX: Add on_few_samples parameter to core rank estimation #13350

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
a8084cb
FIX: Raise exception on rank estimate with too few samples
emmanuel-ferdman Jul 24, 2025
2014510
FIX: Warn on rank estimate with too few samples
emmanuel-ferdman Jul 25, 2025
38184bc
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Jul 25, 2025
b0ae9a2
FIX: Add on_few_samples parameter to core rank estimation
emmanuel-ferdman Aug 6, 2025
3855a24
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Aug 6, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion mne/decoding/tests/test_csp.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -431,7 +431,7 @@ def test_spoc():
# check y
pytest.raises(ValueError, spoc.fit, X, y * 0)

# Check that doesn't take CSP-spcific input
# Check that doesn't take CSP-specific input
pytest.raises(TypeError, SPoC, cov_est="epoch")

# Check mixing matrix on simulated data
Expand Down Expand Up @@ -492,6 +492,7 @@ def test_csp_component_ordering():


@pytest.mark.filterwarnings("ignore:.*Only one sample available.*")
@pytest.mark.filterwarnings("ignore:.*You've got fewer samples than channels.*")
@parametrize_with_checks([CSP(), SPoC()])
def test_sklearn_compliance(estimator, check):
"""Test compliance with sklearn."""
Expand Down
4 changes: 2 additions & 2 deletions mne/rank.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -184,7 +184,7 @@ def _estimate_rank_meeg_signals(
"""
picks_list = _picks_by_type(info)
if data.shape[1] < data.shape[0]:
ValueError(
warn(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From talking to @drammock today, it might actually be safest to have a on_few_samples="raise" option in public-facing functions/APIs that eventually gets passed to this function. Then you can use _on_missing(...) helper function to throw an error or emit a warning. Would you be up for trying this?

Then in the tests below you should be able to pass on_few_samples="ignore" rather than adding a pytest warning ignore decorator

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@larsoner sounds good, I think I got it and implemented it. Please let me know if I missed anything.

"You've got fewer samples than channels, your "
"rank estimate might be inaccurate."
)
Expand Down Expand Up @@ -249,7 +249,7 @@ def _estimate_rank_meeg_cov(
scalings = _handle_default("scalings_cov_rank", scalings)
_apply_scaling_cov(data, picks_list, scalings)
if data.shape[1] < data.shape[0]:
ValueError(
warn(
"You've got fewer samples than channels, your "
"rank estimate might be inaccurate."
)
Expand Down
1 change: 1 addition & 0 deletions mne/simulation/tests/test_evoked.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,7 @@ def test_simulate_evoked():
@pytest.mark.filterwarnings("ignore:No average EEG reference present")
@pytest.mark.filterwarnings("ignore:Too few samples")
@pytest.mark.filterwarnings("ignore:Epochs are not baseline corrected")
@pytest.mark.filterwarnings("ignore:.*You've got fewer samples than channels.*")
def test_add_noise():
"""Test noise addition."""
rng = np.random.default_rng(0)
Expand Down
4 changes: 4 additions & 0 deletions mne/tests/test_cov.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -412,6 +412,7 @@ def _assert_cov(cov, cov_desired, tol=0.005, nfree=True):

@pytest.mark.slowtest
@pytest.mark.parametrize("rank", ("full", None))
@pytest.mark.filterwarnings("ignore:.*You've got fewer samples than channels.*")
def test_cov_estimation_with_triggers(rank, tmp_path):
"""Test estimation from raw with triggers."""
raw = read_raw_fif(raw_fname)
Expand Down Expand Up @@ -781,6 +782,7 @@ def raw_epochs_events():


@pytest.mark.parametrize("rank", (None, "full", "info"))
@pytest.mark.filterwarnings("ignore:.*You've got fewer samples than channels.*")
def test_low_rank_methods(rank, raw_epochs_events):
"""Test low-rank covariance matrix estimation."""
pytest.importorskip("sklearn")
Expand Down Expand Up @@ -814,6 +816,7 @@ def test_low_rank_methods(rank, raw_epochs_events):
assert these_bounds[0] < cov["loglik"] < these_bounds[1], (rank, method)


@pytest.mark.filterwarnings("ignore:.*You've got fewer samples than channels.*")
def test_low_rank_cov(raw_epochs_events):
"""Test additional properties of low rank computations."""
pytest.importorskip("sklearn")
Expand Down Expand Up @@ -890,6 +893,7 @@ def test_low_rank_cov(raw_epochs_events):


@testing.requires_testing_data
@pytest.mark.filterwarnings("ignore:.*You've got fewer samples than channels.*")
def test_cov_ctf():
"""Test basic cov computation on ctf data with/without compensation."""
pytest.importorskip("sklearn")
Expand Down
Loading