Merge branch 'main' into perf/reader-block-sizes

Author: larsoner

doc/changes/dev/14185.newfeature.rst

@@ -0,0 +1 @@
+Add a ``block`` parameter to :func:`mne.viz.plot_source_estimates`, :meth:`mne.SourceEstimate.plot` and :meth:`mne.VolSourceEstimate.plot_3d` to halt execution until the figure is closed, by `Cedric Conday`_.

doc/changes/dev/14245.newfeature.rst

@@ -0,0 +1 @@
+Speed up channel selection and projector construction by replacing linear channel-name lookups with dictionaries, by `Bruno Aristimunha`_.

mne/_fiff/pick.py

@@ -319,13 +319,17 @@ def pick_channels(ch_names, include, exclude=(), ordered=True, *, verbose=None):
         include = list(ch_names)
     if not isinstance(exclude, list):
         exclude = list(exclude)
+    # ch_names is unique (checked above), so a lookup table is safe here; the
+    # list scans this replaces made the loop quadratic in the channel count
+    name_to_idx = {name: ii for ii, name in enumerate(ch_names)}
+    exclude_set = set(exclude)
     sel, missing = list(), list()
     for name in include:
-        if name in ch_names:
-            if name not in exclude:
-                sel.append(ch_names.index(name))
-        else:
+        idx = name_to_idx.get(name)
+        if idx is None:
             missing.append(name)
+        elif name not in exclude_set:
+            sel.append(idx)
     if len(missing) and ordered:
         raise ValueError(
             f"Missing channels from ch_names required by include:\n{missing}"
@@ -1408,13 +1412,20 @@ def _picks_str_to_idx(
     # second: match all to channel names
     #
 
+    # setdefault keeps the first occurrence, so this matches list.index()
+    # exactly even for duplicate names (which are rejected further down, so
+    # the difference is not reachable today -- it just keeps the swap honest)
+    name_to_idx = {}
+    for ii, name in enumerate(info["ch_names"]):
+        name_to_idx.setdefault(name, ii)
     bad_names = []
     picks_name = list()
     for pick in picks:
-        try:
-            picks_name.append(info["ch_names"].index(pick))
-        except ValueError:
+        idx = name_to_idx.get(pick)
+        if idx is None:
             bad_names.append(pick)
+        else:
+            picks_name.append(idx)
 
     #
     # third: match all to types

mne/_fiff/proj.py

@@ -889,11 +889,15 @@ def _make_projector(projs, ch_names, bads=(), include_active=True, inplace=False
             # the projection vectors omitting bad channels
             sel = []
             vecsel = []
-            p_set = set(p["data"]["col_names"])  # faster membership access
+            # map name -> position once; .index() here made this loop quadratic
+            # in the channel count (~16x slower at 306 channels, ~47x at 1000)
+            p_idx = {name: i for i, name in enumerate(p["data"]["col_names"])}
             for c, name in enumerate(ch_names):
-                if name not in bads and name in p_set:
-                    sel.append(c)
-                    vecsel.append(p["data"]["col_names"].index(name))
+                if name not in bads:
+                    vi = p_idx.get(name)
+                    if vi is not None:
+                        sel.append(c)
+                        vecsel.append(vi)
 
             # If there is something to pick, pickit
             nrow = p["data"]["nrow"]

mne/_fiff/tests/test_pick.py

@@ -762,3 +762,28 @@ def test_get_channel_types_equiv(meg, eeg, ordered):
     types = np.array(raw.get_channel_types(picks=picks))
     types_iter = np.array([channel_type(raw.info, idx) for idx in picks])
     assert_array_equal(types, types_iter)
+
+
+def test_pick_channels_matches_by_name():
+    """Test picking maps names to positions regardless of the order given."""
+    ch_names = ["a", "b", "c", "d"]
+    # include is out of order, repeats a name, and names one that is excluded
+    sel = pick_channels(ch_names, ["d", "b", "b", "c"], exclude=["c"], ordered=False)
+    assert_array_equal(sel, [1, 3])
+    # with ordered=True the caller's order is kept, duplicates and all
+    sel = pick_channels(ch_names, ["d", "b", "b"], ordered=True)
+    assert_array_equal(sel, [3, 1, 1])
+    # a name that is not present is an error, not a silent skip
+    with pytest.raises(ValueError, match="Missing channels"):
+        pick_channels(ch_names, ["a", "nope"], ordered=True)
+
+
+def test_picks_to_idx_duplicate_names():
+    """Test a repeated channel name resolves to its first position."""
+    with pytest.warns(RuntimeWarning, match="not unique"):
+        info = create_info(["a", "b", "a"], 100.0, "eeg")
+    # "b" is unambiguous; picking it must not be shifted by the duplicate "a"
+    assert_array_equal(_picks_to_idx(info, ["b"]), [1])
+    # an ambiguous name is rejected rather than silently resolved
+    with pytest.raises(ValueError, match="could not be interpreted"):
+        _picks_to_idx(info, ["a"])

mne/gui/_coreg.py

@@ -65,7 +65,7 @@
     _plot_mri_fiducials,
     _plot_sensors_3d,
 )
-from ..viz.backends._utils import _qt_app_exec, _qt_safe_window
+from ..viz.backends._utils import _qt_block, _qt_safe_window
 from ..viz.utils import safe_event
 
 
@@ -380,8 +380,8 @@ def _get_default(var, val):
         self._trans_modified = False
         self._mri_fids_modified = False
         self._mri_scale_modified = False
-        if block and self._renderer._kind != "notebook":
-            _qt_app_exec(self._renderer.figure.store["app"])
+        if block and self._renderer._kind == "qt":
+            _qt_block(self._renderer.plotter.app_window)
 
     def _set_subjects_dir(self, subjects_dir):
         if subjects_dir is None or not subjects_dir:

mne/gui/_dipolefit.py

@@ -20,15 +20,15 @@
 from ..cov import _ensure_cov, compute_whitener, make_ad_hoc_cov
 from ..dipole import Dipole, fit_dipole
 from ..evoked import Evoked
-from ..forward import convert_forward_solution, make_field_map
+from ..forward import make_field_map
 from ..forward._make_forward import _ForwardModeler
 from ..minimum_norm import apply_inverse, make_inverse_operator
 from ..source_estimate import (
     SourceEstimate,
     _BaseSurfaceSourceEstimate,
     read_source_estimate,
 )
-from ..source_space import setup_volume_source_space
+from ..source_space._source_space import _complete_vol_src, _make_discrete_source_space
 from ..surface import _normal_orth
 from ..transforms import _get_trans, _get_transforms_to_coord_frame, apply_trans
 from ..utils import (
@@ -842,10 +842,6 @@ def _on_dipole_toggle(active, dip_num):
         def _on_dipole_set_name(name, dip_num):
             return self._on_dipole_set_name(name, dip_num)
 
-        @_auto_weakref
-        def _on_dipole_toggle_fix_orientation(fix, dip_num):
-            return self._on_dipole_toggle_fix_orientation(fix, dip_num)
-
         @_auto_weakref
         def _on_dipole_delete(dip_num):
             return self._on_dipole_delete(dip_num)
@@ -881,12 +877,9 @@ def _on_dipole_hover(dip_num, hover):
                 arrow_mesh=arrow_mesh,
                 color=dip_color,
                 dip=dip,
-                fix_ori=True,
-                fix_position=True,
                 helmet_coords=helmet_coords,
                 helmet_pos=helmet_pos,
                 num=dip_num,
-                # fit_time=self._current_time,
             )
             self._dipoles[dip_num] = dipole_dict
 
@@ -924,16 +917,6 @@ def _on_dipole_hover(dip_num, hover):
                 enter=partial(_on_dipole_hover, dip_num=dip_num, hover=True),
                 leave=partial(_on_dipole_hover, dip_num=dip_num, hover=False),
             )
-            widgets.append(
-                r._dock_add_check_box(
-                    name="Fix ori",
-                    value=True,
-                    callback=partial(
-                        _on_dipole_toggle_fix_orientation, dip_num=dip_num
-                    ),
-                    layout=hlayout,
-                )
-            )
             widgets.append(
                 r._dock_add_button(
                     name="",
@@ -1011,58 +994,32 @@ def _fit_timecourses(self):
             # TODO: When two active dipoles have (nearly) identical positions, they
             # collapse to a single point in the discrete source space below, which
             # errors out. Ideal behavior unclear: merge them, or error informatively?
-            this_src = setup_volume_source_space(
-                "sample",
-                pos=dict(
-                    rr=apply_trans(
-                        self._head_mri_t,
-                        np.vstack([d["dip"].pos[0] for d in active_dips]),
-                    ),
-                    nn=apply_trans(
-                        self._head_mri_t,
-                        np.vstack([d["dip"].ori[0] for d in active_dips]),
-                    ),
-                ),
+            this_src = _complete_vol_src(
+                [
+                    _make_discrete_source_space(
+                        pos=dict(
+                            rr=np.vstack([d["dip"].pos[0] for d in active_dips]),
+                            nn=np.vstack([d["dip"].ori[0] for d in active_dips]),
+                        ),
+                        coord_frame="head",
+                    )
+                ]
             )
             this_fwd = self.fwd.compute(this_src)
-            this_fwd = convert_forward_solution(this_fwd, surf_ori=False)
 
             if self._multi_dipole_method == "Multi dipole (MNE)":
                 inv = make_inverse_operator(
-                    self._evoked.info,
-                    # fwd,
-                    this_fwd,
-                    self._cov,
-                    fixed=False,
-                    loose=1.0,
+                    info=self._evoked.info,
+                    forward=this_fwd,
+                    noise_cov=self._cov,
+                    loose=0,
                     depth=0,
                     rank=self._rank,
                 )
-                stc = apply_inverse(
-                    self._evoked,
-                    inv,
-                    method="MNE",
-                    lambda2=1e-6,
-                    pick_ori="vector",
-                )
-
-                timecourses = stc.magnitude().data
-                orientations = (stc.data / timecourses[:, np.newaxis, :]).transpose(
-                    0, 2, 1
-                )
-                fixed_timecourses = stc.project(
-                    np.array([dip["dip"].ori[0] for dip in active_dips])
-                )[0].data
-
+                stc = apply_inverse(self._evoked, inv, method="MNE", lambda2=1e-6)
                 for i, dip in enumerate(active_dips):
-                    if dip["fix_ori"]:
-                        dip["timecourse"] = fixed_timecourses[i]
-                        dip["orientation"] = dip["dip"].ori.repeat(
-                            len(stc.times), axis=0
-                        )
-                    else:
-                        dip["timecourse"] = timecourses[i]
-                        dip["orientation"] = orientations[i]
+                    dip["timecourse"] = stc.data[i]
+                    dip["orientation"] = dip["dip"].ori.repeat(len(stc.times), axis=0)
             else:
                 assert self._multi_dipole_method == "Single dipole"  # only other option
                 for dip in active_dips:
@@ -1071,20 +1028,16 @@ def _fit_timecourses(self):
                         self._cov,
                         self._bem,
                         pos=dip["dip"].pos[0],  # position is always fixed
-                        ori=dip["dip"].ori[0] if dip["fix_ori"] else None,
+                        ori=dip["dip"].ori[0],
                         trans=self._head_mri_t,
                         rank=self._rank,
                         n_jobs=self._n_jobs,
                         verbose=True,
                     )
-                    if dip["fix_ori"]:
-                        dip["timecourse"] = dip_with_timecourse.data[0]
-                        dip["orientation"] = dip["dip"].ori.repeat(
-                            len(dip_with_timecourse.times), axis=0
-                        )
-                    else:
-                        dip["timecourse"] = dip_with_timecourse.amplitude
-                        dip["orientation"] = dip_with_timecourse.ori
+                    dip["timecourse"] = dip_with_timecourse.data[0]
+                    dip["orientation"] = dip["dip"].ori.repeat(
+                        len(dip_with_timecourse.times), axis=0
+                    )
 
             # Update matplotlib canvas at the bottom of the window. Timecourses are
             # stored in SI units (Am), but shown in nAm, hence the 1e9 scaling at the
@@ -1261,7 +1214,11 @@ def _update_arrows(self):
     # TODO: Need to expose a public method for setting the multi-dipole method
     def _on_select_method(self, method):
         """Select the method to use for multi-dipole timecourse fitting."""
-        _check_option("method", method, ("Multi dipole (MNE)", "Single dipole"))
+        _check_option(
+            "method",
+            method,
+            ("Multi dipole (MNE)", "Single dipole"),
+        )
         if method == self._multi_dipole_method:
             return
         self._multi_dipole_method = method
@@ -1297,11 +1254,6 @@ def _on_dipole_set_name(self, name, dip_num):
         self._dipoles[dip_num]["dip"].name = name
         self._renderer._mplcanvas.update_plot()
 
-    def _on_dipole_toggle_fix_orientation(self, fix, dip_num):
-        """Fix dipole orientation when fitting timecourse."""
-        self._dipoles[dip_num]["fix_ori"] = bool(fix)
-        self._fit_timecourses()
-
     def _on_dipole_delete(self, dip_num):
         """Delete previously fitted dipole."""
         dipole = self._dipoles[dip_num]

mne/gui/tests/test_dipolefit.py

@@ -303,7 +303,7 @@ def test_dipolefit_gui_dipole_controls(
         g._set_camera_preset("Sideways")
 
     # Test toggling dipoles off and on. This is done through the GUI widgets, which are
-    # ordered: [active, name, fix orientation, delete].
+    # ordered: [active, name, delete].
     dip = mne.read_dipole(fname_dip)[[12, 15]]  # 80ms and 90ms
     g.add_dipole(dip, name=["rh", "lh"])
     dip1, dip2 = g._dipoles.values()
@@ -354,17 +354,6 @@ def test_dipolefit_gui_dipole_controls(
     new_timecourses = np.vstack((dip1["timecourse"], dip2["timecourse"]))
     assert np.allclose(old_timecourses, new_timecourses, atol=0)
 
-    # Toggle fixed orientation off and on.
-    assert dip1["fix_ori"] and dip2["fix_ori"]
-    dip1["widgets"][2].set_value(False)
-    assert not dip1["fix_ori"]
-    new_timecourses = np.vstack((dip1["timecourse"], dip2["timecourse"]))
-    assert not np.allclose(old_timecourses, new_timecourses, atol=1e-9)
-    dip1["widgets"][2].set_value(True)
-    assert dip1["fix_ori"]
-    new_timecourses = np.vstack((dip1["timecourse"], dip2["timecourse"]))
-    assert np.allclose(old_timecourses, new_timecourses, atol=0)
-
     # Change the names of the dipoles.
     dip1["widgets"][1].set_value("dipole1")
     g._on_dipole_set_name("dipole2", dip2["num"])
@@ -373,7 +362,7 @@ def test_dipolefit_gui_dipole_controls(
 
     # Remove a dipole (through the "delete" button).
     line, dot = dip1["line_artist"], dip1["dot_artist"]
-    dip1["widgets"][3].set_value(None)
+    dip1["widgets"][2].set_value(None)
     assert line not in g._renderer._mplcanvas.axes.lines
     assert dot not in g._renderer._mplcanvas.axes.lines
     assert len(g.dipoles) == 1
@@ -385,17 +374,6 @@ def test_dipolefit_gui_dipole_controls(
     assert list(g._dipoles.keys())[1] == 2
     assert list(g._dipoles.values())[1]["num"] == 2  # new dipole number
 
-    # Fitting the timecourse of a single dipole, with a free orientation.
-    g._on_dipole_toggle(False, 2)  # only leave a single dipole active
-    g._on_select_method("Single dipole")
-    g._renderer._process_events()  # run the deferred refit
-    assert dip2["fix_ori"]
-    assert_allclose(dip2["orientation"], dip2["dip"].ori.repeat(len(evoked.times), 0))
-    g._on_dipole_toggle_fix_orientation(False, dip2["num"])
-    assert not dip2["fix_ori"]
-    assert dip2["orientation"].shape == (len(evoked.times), 3)
-    assert not np.allclose(dip2["orientation"][0], dip2["orientation"][-1], atol=1e-9)
-
     g.close()
 
 

mne/source_estimate.py

@@ -778,6 +778,7 @@ def plot(
         view_layout="vertical",
         add_data_kwargs=None,
         brain_kwargs=None,
+        block=False,
         verbose=None,
     ):
         from .viz import plot_source_estimates
@@ -813,6 +814,7 @@ def plot(
             view_layout=view_layout,
             add_data_kwargs=add_data_kwargs,
             brain_kwargs=brain_kwargs,
+            block=block,
             verbose=verbose,
         )
         return brain
@@ -2399,6 +2401,7 @@ def plot_3d(
         view_layout="vertical",
         add_data_kwargs=None,
         brain_kwargs=None,
+        block=False,
         verbose=None,
     ):
         return super().plot(
@@ -2431,6 +2434,7 @@ def plot_3d(
             view_layout=view_layout,
             add_data_kwargs=add_data_kwargs,
             brain_kwargs=brain_kwargs,
+            block=block,
             verbose=verbose,
         )