Skip to content
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

Jump to bottom

Allow making inverse solutions from fixed-orientation discrete forward models, enabling multi-dipole modeling #10464

Merged
merged 13 commits into from
Apr 20, 2022
Merged

Allow making inverse solutions from fixed-orientation discrete forward models, enabling multi-dipole modeling #10464

merged 13 commits into from
Apr 20, 2022

Conversation

wmvanvliet
Copy link
Contributor

@wmvanvliet wmvanvliet commented Mar 29, 2022 •
edited by larsoner
Loading

Picking this up from #10440.

MNE-Python has most of the ingredients for Xfit-style multi-dipole modeling. This PR removes the last barrier in the way of building a multi-dipole inverse solution: allowing the building of inverse models for the type of forward models produced by make_forward_dipole, namely fixed-orientation, discrete forward models.

Todo:

  • Allow fixed-orientation discrete forward models in _prepare_forward
  • Add unit test for making an inverse operator from a fixed-orientation discrete forward model
  • Make example of multi-dipole modeling
  • Make make_forward_dipole take a list of dipoles to prevent having to manually concatenate them
  • Update What's New

@wmvanvliet wmvanvliet added the ENH label Mar 29, 2022
@wmvanvliet wmvanvliet mentioned this pull request Mar 29, 2022
Closed
agramfort
agramfort approved these changes Mar 30, 2022
View reviewed changes
Copy link
Member

@agramfort agramfort left a comment

Choose a reason for hiding this comment

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

Looks neat

https://output.circle-artifacts.com/output/job/fdadba28-77a8-44ce-8d91-3fbe4e36cadb/artifacts/0/dev/auto_examples/inverse/multi_dipole_model.html#sphx-glr-auto-examples-inverse-multi-dipole-model-py

I am also think it's probably too much code to write for most users but it's a great step forward.

@wmvanvliet
Copy link
Contributor Author

Thanks!

I am also think it's probably too much code to write for most users but it's a great step forward.

Yeah, I went perhaps a little overboard on the figures. Also, guided dipole fitting using code is not the best way. You should really be using Xfit for that. However, constructing the actual multi-dipole model once you have the dipoles is not that bad.

@agramfort
Copy link
Member

@wmvanvliet can you see why CIs complain?

@wmvanvliet
Copy link
Contributor Author

Qt somehow broke, but I don't think that is related to the changes I've been making.

@wmvanvliet
Copy link
Contributor Author

@wmvanvliet can you see why CIs complain?

There were some unit tests that explicitly tested for errors throws when computing fixed orientation inverses from discrete forward models. I've removed them.

@agramfort
Copy link
Member

agramfort commented Apr 19, 2022 via email

@wmvanvliet
Copy link
Contributor Author

I think I'm happy!

@larsoner
Merge remote-tracking branch 'upstream/main' into multi-dipole-modeling
ae3f9f8 
* upstream/main: (40 commits)
  FIX: Flake (mne-tools#10540)
  FIX: Correct link (mne-tools#10536)
  DOC: Update installers (mne-tools#10535)
  ENH: Add dark mode to website (mne-tools#10523)
  WIP: Copy BEM surfaces by default (don't symlink) (mne-tools#10531)
  Avoid lowpass=0 in brainvision data (mne-tools#10517)
  DOC: Update installers [skip azp] [skip actions] (mne-tools#10528)
  FIX: Fix for old build (mne-tools#10527)
  Fix line noise at wrong frequencies (mne-tools#10525)
  FIX : read fids in eeglab (mne-tools#10521)
  MAINT: Prefer PySide6 in testing (mne-tools#10513)
  ENH: Add overview_mode support (mne-tools#10501)
  MRG: Updates for qtpy in mne-qt-browser (mne-tools#10509)
  BUG: Fix bug with themes on macOS (mne-tools#10500)
  MAINT: Bump installer links (mne-tools#10511)
  Add metadata to combine_channels (mne-tools#10504)
  MAINT: Standardize tests (mne-tools#10502)
  CI: Test circle (mne-tools#10506)
  ENH: Use HiDPI splash screen on HiDPI screens (mne-tools#10503)
  WIP,MNT: Add support for QtPy (mne-tools#10430)
  ...
@larsoner
Copy link
Member

I merged with upstream/main to fix the CircleCI error, but it looks like latest.inc is still missing

larsoner
larsoner reviewed Apr 19, 2022
View reviewed changes
Copy link
Member

@larsoner larsoner left a comment

Choose a reason for hiding this comment

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

Looks great other than some minor comments!

mne/forward/forward.py Outdated Show resolved Hide resolved
examples/inverse/multi_dipole_model.py Outdated Show resolved Hide resolved
examples/inverse/multi_dipole_model.py Outdated Show resolved Hide resolved
examples/inverse/multi_dipole_model.py Outdated Show resolved Hide resolved
mne/inverse_sparse/tests/test_gamma_map.py Show resolved Hide resolved
mne/minimum_norm/inverse.py Outdated Show resolved Hide resolved
mne/utils/docs.py Show resolved Hide resolved
@wmvanvliet
Copy link
Contributor Author

it looks like latest.inc is still missing

I like to put that in at the very end, since it produces so many merge conflicts

@larsoner larsoner mentioned this pull request Apr 20, 2022
Merged
@larsoner larsoner merged commit 8233041 into mne-tools:main Apr 20, 2022
@larsoner
Copy link
Member

Thanks @wmvanvliet !

@wmvanvliet
Copy link
Contributor Author

Pushed latest.inc directly to main.

larsoner added a commit to agramfort/mne-python that referenced this pull request Apr 21, 2022
@larsoner
Merge remote-tracking branch 'upstream/main' into enh/crop_by_annot
a5a3ec0 
* upstream/main: (52 commits)
  MAINT: Extra test for coreg (mne-tools#10549)
  BUG: Fix annot meas_date / crop (mne-tools#10491)
  Update latest.inc
  Use liblinear solver instead of lbgfs in all decodung examples [skip azp][skip actions] (mne-tools#10552)
  STY: Hotfix for HTML [ci skip]
  Allow making inverse solutions from fixed-orientation discrete forward models, enabling multi-dipole modeling (mne-tools#10464)
  Correct documented default number of CV splits [skip azp][skip actions] (mne-tools#10548)
  MRG: Add error checking to prevent the creation of montage with invalid [x, y, z] (mne-tools#10547)
  MRG: Add "verbose" parameter to pick_channels() method (mne-tools#10544)
  CI: Avoid bad PySide6 (mne-tools#10545)
  FIX: Flake (mne-tools#10540)
  FIX: Correct link (mne-tools#10536)
  DOC: Update installers (mne-tools#10535)
  ENH: Add dark mode to website (mne-tools#10523)
  WIP: Copy BEM surfaces by default (don't symlink) (mne-tools#10531)
  Avoid lowpass=0 in brainvision data (mne-tools#10517)
  DOC: Update installers [skip azp] [skip actions] (mne-tools#10528)
  FIX: Fix for old build (mne-tools#10527)
  Fix line noise at wrong frequencies (mne-tools#10525)
  FIX : read fids in eeglab (mne-tools#10521)
  ...
larsoner added a commit to alexrockhill/mne-python that referenced this pull request Apr 21, 2022
@larsoner
Merge remote-tracking branch 'upstream/main' into muscle
41da91f 
* upstream/main:
  MAINT: Extra test for coreg (mne-tools#10549)
  BUG: Fix annot meas_date / crop (mne-tools#10491)
  Update latest.inc
  Use liblinear solver instead of lbgfs in all decodung examples [skip azp][skip actions] (mne-tools#10552)
  STY: Hotfix for HTML [ci skip]
  Allow making inverse solutions from fixed-orientation discrete forward models, enabling multi-dipole modeling (mne-tools#10464)
larsoner added a commit to hoechenberger/mne-python that referenced this pull request Apr 21, 2022
@larsoner
Merge remote-tracking branch 'upstream/main' into hoechenberger/issue…
b19ee5f 
…10520

* upstream/main:
  MAINT: Extra test for coreg (mne-tools#10549)
  BUG: Fix annot meas_date / crop (mne-tools#10491)
  Update latest.inc
  Use liblinear solver instead of lbgfs in all decodung examples [skip azp][skip actions] (mne-tools#10552)
  STY: Hotfix for HTML [ci skip]
  Allow making inverse solutions from fixed-orientation discrete forward models, enabling multi-dipole modeling (mne-tools#10464)
  Correct documented default number of CV splits [skip azp][skip actions] (mne-tools#10548)
  MRG: Add error checking to prevent the creation of montage with invalid [x, y, z] (mne-tools#10547)
  MRG: Add "verbose" parameter to pick_channels() method (mne-tools#10544)
  CI: Avoid bad PySide6 (mne-tools#10545)
@wmvanvliet wmvanvliet deleted the multi-dipole-modeling branch September 4, 2023 07:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@larsoner larsoner larsoner left review comments

@agramfort agramfort agramfort approved these changes

Assignees
No one assigned
Labels
ENH
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@wmvanvliet @agramfort @larsoner