From f62a168a45a86f859cc11e71adebb38a0989f971 Mon Sep 17 00:00:00 2001 From: Marijn van Vliet Date: Fri, 28 Aug 2026 15:42:42 +0200 Subject: [PATCH 1/2] XFIT: remove rotating dipoles and make orientations more accurate --- mne/gui/_dipolefit.py | 106 ++++++++++++------------------------------ 1 file changed, 29 insertions(+), 77 deletions(-) diff --git a/mne/gui/_dipolefit.py b/mne/gui/_dipolefit.py index 7a1d26b6877..93f403fc4ec 100644 --- a/mne/gui/_dipolefit.py +++ b/mne/gui/_dipolefit.py @@ -20,7 +20,7 @@ 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 ( @@ -28,7 +28,7 @@ _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 ( @@ -835,10 +835,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) @@ -874,12 +870,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 @@ -917,16 +910,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="", @@ -1004,58 +987,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: @@ -1064,20 +1021,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 @@ -1254,7 +1207,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 @@ -1290,11 +1247,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] From f6df73f153caaadc3997bf76ace826f6a56e0147 Mon Sep 17 00:00:00 2001 From: Marijn van Vliet Date: Fri, 28 Aug 2026 15:47:42 +0200 Subject: [PATCH 2/2] update unit tests --- mne/gui/tests/test_dipolefit.py | 26 ++------------------------ 1 file changed, 2 insertions(+), 24 deletions(-) diff --git a/mne/gui/tests/test_dipolefit.py b/mne/gui/tests/test_dipolefit.py index e6543747da3..0be55e6384a 100644 --- a/mne/gui/tests/test_dipolefit.py +++ b/mne/gui/tests/test_dipolefit.py @@ -299,7 +299,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() @@ -350,17 +350,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"]) @@ -369,7 +358,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 @@ -381,17 +370,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()