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

ENH: Allow gradient compensated data in maxwell_filter #10554

Merged
merged 5 commits into from
Apr 14, 2023
Merged

ENH: Allow gradient compensated data in maxwell_filter #10554

merged 5 commits into from
Apr 14, 2023

Conversation

larsoner
Copy link
Member

I was interested to see if we could add support for maxwell_filter(raw_ctf.apply_gradient_compensation(3)). It looks like it's possible, but the performance is not as good as processing un-compensated data. Maybe I'm doing something wrong, though...

I'll leave this open and think about it for a bit.

@hoechenberger hoechenberger added this to the 1.2 milestone Jun 10, 2022
@larsoner larsoner modified the milestones: 1.2, 1.3 Sep 13, 2022
@larsoner larsoner removed this from the 1.3 milestone Dec 8, 2022
@larsoner
Copy link
Member Author

larsoner commented Apr 6, 2023

@britta-wstnr I hear you have been trying this :) Think it's working well enough to merge?

@larsoner larsoner added this to the 1.4 milestone Apr 6, 2023
@britta-wstnr
Copy link
Member

Mhhh ... I agree with you that it's not making much of a difference whether 3rd order gradients are applied or not ... which is also puzzling to me (as it does make quite a difference on the pre-tSSS data). Let me investigate a little more next week 🕵️ only got to check it out on one of our datasets so far!

@larsoner larsoner removed this from the 1.4 milestone Apr 6, 2023
@britta-wstnr
Copy link
Member

I extensively tested this on 50 CTF datasets with varying amounts of noise/contamination.

My findings:

  • the 3rd order synthetic gradient does not make much of a difference - probably a lot of the same noise gets caught by the tSSS as well?
  • a comparison between tSSS -> gradient compensation and gradient compensation -> tSSS (this PR) shows that the results are really similar (for most cases almost identical), except that what this PR implements seems to result in a little less noise overall.

@larsoner
Copy link
Member Author

Okay great, in it goes then!

@larsoner larsoner merged commit fc981bd into mne-tools:main Apr 14, 2023
@larsoner larsoner deleted the gradcomp branch April 14, 2023 14:59
larsoner added a commit to cbrnr/mne-python that referenced this pull request Apr 21, 2023
@larsoner
Merge remote-tracking branch 'upstream/main' into precommit
1770b7b 
* upstream/main: (50 commits)
  BUG: Fix bug with paths (mne-tools#11639)
  MAINT: Report download time and size (mne-tools#11635)
  MRG: Allow retrieval of channel names via make_1020_channel_selections() (mne-tools#11632)
  Fix index name in to_data_frame()'s docstring (mne-tools#11457)
  MAINT: Use VTK prerelease wheels in pre jobs (mne-tools#11629)
  ENH: Allow gradient compensated data in maxwell_filter (mne-tools#10554)
  make test compatible with future pandas (mne-tools#11625)
  Display SVG figures correctly in Report (mne-tools#11623)
  API: Port ieeg gui over to mne-gui-addons and add tfr gui example (mne-tools#11616)
  MAINT: Add token [ci skip] (mne-tools#11622)
  API: One cycle of backward compat (mne-tools#11621)
  MAINT: Use git rather than zipball (mne-tools#11620)
  ENH: Speed up code a bit (mne-tools#11614)
  [BUG, MRG] Don't modify info in place for transform points (mne-tools#11612)
  [BUG, MRG] Fix topomap extra plot generated, add util to check a range (mne-tools#11607)
  ENH: Add mne-bids-pipeline to mne sys_info (mne-tools#11606)
  MAINT: `coding: utf-8` is implicit in Python 3 (mne-tools#11599)
  ENH: Read eyetracking data (Eyelink) (Fork of mne-tools#10855 ) (mne-tools#11152)
  MAINT: In Python 3, do not prefix literals with `u` (mne-tools#11604)
  MAINT: object is an implicit base for all classes (mne-tools#11601)
  ...
larsoner added a commit to georgeoneill/mne-python that referenced this pull request Apr 21, 2023
@larsoner
Merge remote-tracking branch 'upstream/main' into hfc
c55101f 
* upstream/main:
  BUG: Fix bug with paths (mne-tools#11639)
  MAINT: Report download time and size (mne-tools#11635)
  MRG: Allow retrieval of channel names via make_1020_channel_selections() (mne-tools#11632)
  Fix index name in to_data_frame()'s docstring (mne-tools#11457)
  MAINT: Use VTK prerelease wheels in pre jobs (mne-tools#11629)
  ENH: Allow gradient compensated data in maxwell_filter (mne-tools#10554)
  make test compatible with future pandas (mne-tools#11625)
larsoner added a commit to larsoner/mne-python that referenced this pull request Apr 25, 2023
@larsoner
Merge remote-tracking branch 'upstream/main' into cola5
7c44d72 
* upstream/main: (152 commits)
  FIX: missing channels/fiducials can be np.nan (mne-tools#11634)
  use py3.10 in precommit config (mne-tools#11648)
  MAINT: Unify GH Actions pytest (mne-tools#11644)
  MRG: Rename "Discourse" link in top navigation to "Forum" [ci skip] (mne-tools#11649)
  ENH: Add support for Harmonic Field correction (mne-tools#11536)
  Add pre-commit (mne-tools#11541)
  BUG: Fix bug with paths (mne-tools#11639)
  MAINT: Report download time and size (mne-tools#11635)
  MRG: Allow retrieval of channel names via make_1020_channel_selections() (mne-tools#11632)
  Fix index name in to_data_frame()'s docstring (mne-tools#11457)
  MAINT: Use VTK prerelease wheels in pre jobs (mne-tools#11629)
  ENH: Allow gradient compensated data in maxwell_filter (mne-tools#10554)
  make test compatible with future pandas (mne-tools#11625)
  Display SVG figures correctly in Report (mne-tools#11623)
  API: Port ieeg gui over to mne-gui-addons and add tfr gui example (mne-tools#11616)
  MAINT: Add token [ci skip] (mne-tools#11622)
  API: One cycle of backward compat (mne-tools#11621)
  MAINT: Use git rather than zipball (mne-tools#11620)
  ENH: Speed up code a bit (mne-tools#11614)
  [BUG, MRG] Don't modify info in place for transform points (mne-tools#11612)
  ...
larsoner added a commit to larsoner/mne-python that referenced this pull request Apr 25, 2023
@larsoner
Merge remote-tracking branch 'upstream/main' into kernel
bbc55af 
* upstream/main: (117 commits)
  FIX: missing channels/fiducials can be np.nan (mne-tools#11634)
  use py3.10 in precommit config (mne-tools#11648)
  MAINT: Unify GH Actions pytest (mne-tools#11644)
  MRG: Rename "Discourse" link in top navigation to "Forum" [ci skip] (mne-tools#11649)
  ENH: Add support for Harmonic Field correction (mne-tools#11536)
  Add pre-commit (mne-tools#11541)
  BUG: Fix bug with paths (mne-tools#11639)
  MAINT: Report download time and size (mne-tools#11635)
  MRG: Allow retrieval of channel names via make_1020_channel_selections() (mne-tools#11632)
  Fix index name in to_data_frame()'s docstring (mne-tools#11457)
  MAINT: Use VTK prerelease wheels in pre jobs (mne-tools#11629)
  ENH: Allow gradient compensated data in maxwell_filter (mne-tools#10554)
  make test compatible with future pandas (mne-tools#11625)
  Display SVG figures correctly in Report (mne-tools#11623)
  API: Port ieeg gui over to mne-gui-addons and add tfr gui example (mne-tools#11616)
  MAINT: Add token [ci skip] (mne-tools#11622)
  API: One cycle of backward compat (mne-tools#11621)
  MAINT: Use git rather than zipball (mne-tools#11620)
  ENH: Speed up code a bit (mne-tools#11614)
  [BUG, MRG] Don't modify info in place for transform points (mne-tools#11612)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@britta-wstnr britta-wstnr britta-wstnr approved these changes

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

Successfully merging this pull request may close these issues.
3 participants
@larsoner @britta-wstnr @hoechenberger