-
-
Notifications
You must be signed in to change notification settings - Fork 1.5k
Warnings for big bems. #3374
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.
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
Warnings for big bems. #3374
Conversation
Current coverage is 86.67%@@ master #3374 diff @@
==========================================
Files 335 335
Lines 57628 57644 +16
Methods 0 0
Messages 0 0
Branches 8721 8784 +63
==========================================
+ Hits 30105 49961 +19856
+ Misses 25694 5006 -20688
- Partials 1829 2677 +848
|
mne/bem.py
Outdated
| else: | ||
| raise RuntimeError('Only 1- or 3-layer BEM computations supported') | ||
| if bem['surfs'][0]['np'] > 10000: | ||
| warn('The bem surface has %s data points. 5120 should be enough.' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should probably warn that it might not save properly, and to DRY it this can be a _check_bem_size private function
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I suppose that is only a problem with three layer bem. So maybe warn only then.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Or maybe it saves to separate files, right? I'll check.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
3-layer BEM gets saved to a single file, just like 1-layer BEM
mne/bem.py
Outdated
| if bem['surfs'][0]['np'] > 10000: | ||
| warn('The bem surface has %s data points. 5120 should be enough.' | ||
| % bem['surfs'][0]['np']) | ||
| _check_bem_size(bem) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
to save users some pain, you could also put this warning in make_bem_model, the lighter-weight step they likely use before making the solution
|
Comments addressed. |
|
thx @jaeilepp |
* 'master' of git://github.com/mne-tools/mne-python: (48 commits) FIX: Flake fix pep8 [MRG] FIX Topographic plotting for KIT-UMD data (mne-tools#3349) QUICKFIX: logging test Error message for simulate evoked. (mne-tools#3372) Warnings for big bems. (mne-tools#3374) Add test for baseline correction. Use apply_baseline in read_evokeds. Makes (None, 0) the default value baseline in apply_baseline. [FIX] Adds apply_baseline to Evoked fix component inds in ica tutorial (mne-tools#3379) FIX: Xdawn with shuffled epochs (mne-tools#3373) FIX consistency: Epochs.load_data() should always return self (mne-tools#3376) MRG: Build relevant examples (mne-tools#3365) [MRG] set vmin, vmax after smoothing in plot_epochs_image (mne-tools#3360) [FIX] Attempt to fix circle. fixing a bg plotting bug for topo plots Modified test and tutorial. [MRG] convert surface using python. (mne-tools#3273) [MRG] raw.plot_psd() with flat channel (mne-tools#3342) ...
Closes #3112