MRG: BEM model and solution in Python #2193

larsoner · 2015-06-08T23:11:35Z

Closes #2190.

~~I translated all the code but haven't tested it at all. In fact I know it will not work. But the current plan for usage is something like this:~~

from mne import make_bem_model, make_bem_solution
subject = 'sample'
model = make_bem_model(subject)  # puts surfaces and conductances together
solution = make_bem_solution(model)  # does the potential computations

agramfort · 2015-06-09T08:35:09Z

mne/bem.py

+    triple = np.dot(cross, y3)
+
+    # XXX refactor with _get_solids?
+    l1 = np.sqrt((y1 * y1).sum())


(y1 * y1).sum() -> np.dot(y1, y1)

fix below too.

Also you use use functions from math module for floats: sqrt, abs ...

yeah, the reason I have it in this form is that I am almost certainly going to vectorize the operation, in which case having it in the sum form will be more convenient to start translating to higher dimensions

larsoner · 2015-06-09T22:08:53Z

It seems to be working for me now, but it is horribly slow. Hopefully I can speed it up tomorrow.

dgwakeman · 2015-06-10T14:21:37Z

You are awesome! I am so excited for this!

larsoner · 2015-06-10T19:10:15Z

FYI I added a couple more files to the testing dataset. BEMs have now been created for ico-2 surfaces (320 instead of 5120 or 1280) which is much faster.

The code at this point seems to work for homog 1-layer, I just need to track down a bug in the 3-layer case. Speed isn't too much worse than C for small shells (<= 1280) and actually faster for standard size (5120) because, while calculating the matrix coefficients is slower in Python, the inversion is much faster:

ico       type    layers  time
2 (320)   C       1       0.0300931930542
2 (320)   Python  1       0.250679969788
2 (320)   C       3       0.347531795502
2 (320)   Python  3       2.41311502457
3 (1280)  C       1       0.621548891068
3 (1280)  Python  1       2.16235113144
3 (1280)  C       3       12.2742669582
3 (1280)  Python  3       21.4681899548
4 (5120)  C       1       24.4842910767
4 (5120)  Python  1       27.2853899002
4 (5120)  C       3       643.578150034
4 (5120)  Python  3       282.933717012

Just need to track down where that bug is, which I probably won't get to until next week.

larsoner · 2015-07-07T17:25:38Z

Okay, 1- and 3-layer model and solution computation works now. Ready for review/merge from my end.

larsoner · 2015-07-09T13:32:04Z

Ping @agramfort @dengemann this is ready for review

dengemann · 2015-07-09T13:44:47Z

@Eric89GXL amazing! How fast is it compared to the original code?

larsoner · 2015-07-09T13:54:29Z

See comment above, faster for normal use case (3-layer 5120)

larsoner · 2015-07-09T13:58:22Z

There might be additional ways of optimizing it, too, if there is interest

dengemann · 2015-07-09T13:59:00Z

here might be additional ways of optimizing it, too, if there is interest

I'm already very impressed. Eric what do we need here, any testing desired?

larsoner · 2015-07-09T13:59:46Z

Hopefully the unit tests are sufficient to make us confident in using it. But sure, if you want to test it out and make comments, it's welcome.

dengemann · 2015-07-09T14:02:13Z

But sure, if you want to test it out and make comments, it's welcome.

I will trust you if you're confident ;)

dengemann · 2015-07-09T14:02:38Z

mne/bem.py

+#
+
+
+def _calc_beta(rk, rk1):


short comment

That would require (re-)understanding what it does first :) but seriously, I'll add one. It also reminds me that I need to check for duplication of functionality across the _compute_forward module, I think these are different but I should check.

larsoner · 2015-07-09T14:03:17Z

Check out the tests, hopefully those will make us confident :) If they don't, then they can be expanded.

dengemann · 2015-07-09T14:10:34Z

mne/bem.py

+      * This is the general multilayer case
+
+    """
+    from scipy import linalg


why import here?

we nest certain scipy imports now to save on import speed, see mne/tests/test_import_nesting.py

right! thanks

larsoner · 2015-07-09T16:31:21Z

@dengemann comments addressed. FYI you asked about speed, and that made me optimize a bit more:

2 (320)  1       C       0.283627033234
2 (320)  1       Python  0.291443109512
2 (320)  3       C       0.465950012207
2 (320)  3       Python  2.45959687233
3 (1280)  1       C       0.660896062851
3 (1280)  1       Python  1.62492084503
3 (1280)  3       C       12.4721641541
3 (1280)  3       Python  11.3650319576
4 (5120)  1       C       24.9352688789
4 (5120)  1       Python  11.2639200687
4 (5120)  3       C       644.138396025
4 (5120)  3       Python  118.89906311

dengemann · 2015-07-09T16:46:38Z

Great!!! I will recompute all my BEMs to enjoy the speedy drive :)

On 09 Jul 2015, at 18:31, Eric Larson notifications@github.com wrote:

@dengemann comments addressed. FYI you asked about speed, and that made me optimize a bit more:

2 (320) 1 C 0.283627033234
2 (320) 1 Python 0.291443109512
2 (320) 3 C 0.465950012207
2 (320) 3 Python 2.45959687233
3 (1280) 1 C 0.660896062851
3 (1280) 1 Python 1.62492084503
3 (1280) 3 C 12.4721641541
3 (1280) 3 Python 11.3650319576
4 (5120) 1 C 24.9352688789
4 (5120) 1 Python 11.2639200687
4 (5120) 3 C 644.138396025
4 (5120) 3 Python 118.89906311
—
Reply to this email directly or view it on GitHub.

agramfort · 2015-07-13T19:23:19Z

mne/bem.py

+    v2 = tri_rr[np.newaxis, 1, :] - fros
+    v3 = tri_rr[np.newaxis, 2, :] - fros
+    triples = _fast_cross_nd_sum(v1, v2, v3)
+    l1 = np.sqrt(np.sum(v1 * v1, axis=1))


is it faster than np.linalg.norm( ..., axis=1) ?

Yes sadly (and axis is only available on 1.9+), I tested both. I think it depends on your application. Probably for larger matrices norm(..., axis) would be faster, we'll have to decide on a case-by-case basis.

agramfort · 2015-07-13T19:30:49Z

please update what's new, rebase and +1 for merge !

thanks awesome !

larsoner · 2015-07-13T19:43:27Z

Rebased and updated, I'll merge once Travis comes back happy

agramfort · 2015-07-13T19:51:20Z

Sounds good

MRG: BEM model and solution in Python

larsoner · 2015-07-13T20:01:54Z

Thanks for the reviews @dengemann @agramfort

dengemann · 2015-07-13T23:07:02Z

Super cool, Eric -- amazing!

On Mon, Jul 13, 2015 at 10:01 PM, Eric Larson notifications@github.com
wrote:

Thanks for the reviews @dengemann https://github.com/dengemann
@agramfort https://github.com/agramfort

—
Reply to this email directly or view it on GitHub
#2193 (comment)
.

agramfort reviewed Jun 9, 2015
View reviewed changes

larsoner force-pushed the bem branch from d1eb3db to a26cc44 Compare June 10, 2015 17:37

larsoner mentioned this pull request Jun 10, 2015

[MRG] Mne watershed bem #2194

Merged

larsoner modified the milestone: 0.10 Jun 16, 2015

larsoner force-pushed the bem branch from 66e2575 to 0290f8e Compare July 7, 2015 17:23

larsoner changed the title ~~WIP: BEM model and solution in Python~~ MRG: BEM model and solution in Python Jul 7, 2015

dengemann reviewed Jul 9, 2015
View reviewed changes

agramfort reviewed Jul 13, 2015
View reviewed changes

larsoner added 4 commits July 13, 2015 15:41

WIP: BEM model and solution in Python [ci skip]

e4bf2ad

FIX: Working

e7ab31e

FIX: Fix comment [ci skip]

a61885d

FIX: Fix test

653ac21

larsoner added 10 commits July 13, 2015 15:41

FIX: Many fixes

ffca031

FIX: Fix path

d851b7c

FIX: Better test

996058a

DOC: Update doc [ci skip]

a666dde

FIX: Fix test

0650156

FIX: Fix from rebase

dddd878

ENH: Speed up calculation

b0635e4

FIX: Better coverage

ee793b6

FIX: Fix nesting

45ecefb

FIX: Update whats new

c46b2cf

larsoner force-pushed the bem branch from 6dcdbec to c46b2cf Compare July 13, 2015 19:43

larsoner added a commit that referenced this pull request Jul 13, 2015

Merge pull request #2193 from Eric89GXL/bem

f60a18a

MRG: BEM model and solution in Python

larsoner merged commit f60a18a into mne-tools:master Jul 13, 2015

larsoner deleted the bem branch July 13, 2015 20:01

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

MRG: BEM model and solution in Python #2193

MRG: BEM model and solution in Python #2193

larsoner commented Jun 8, 2015

agramfort Jun 9, 2015

agramfort Jun 9, 2015

larsoner Jun 9, 2015

larsoner commented Jun 9, 2015

dgwakeman commented Jun 10, 2015

larsoner commented Jun 10, 2015

larsoner commented Jul 7, 2015

larsoner commented Jul 9, 2015

dengemann commented Jul 9, 2015

larsoner commented Jul 9, 2015

larsoner commented Jul 9, 2015

dengemann commented Jul 9, 2015

larsoner commented Jul 9, 2015

dengemann commented Jul 9, 2015

dengemann Jul 9, 2015

larsoner Jul 9, 2015

larsoner commented Jul 9, 2015

dengemann Jul 9, 2015

larsoner Jul 9, 2015

dengemann Jul 9, 2015

larsoner commented Jul 9, 2015

dengemann commented Jul 9, 2015

agramfort Jul 13, 2015

larsoner Jul 13, 2015

agramfort commented Jul 13, 2015

larsoner commented Jul 13, 2015

agramfort commented Jul 13, 2015 via email

larsoner commented Jul 13, 2015

dengemann commented Jul 13, 2015

MRG: BEM model and solution in Python #2193

MRG: BEM model and solution in Python #2193

Conversation

larsoner commented Jun 8, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

larsoner commented Jun 9, 2015

dgwakeman commented Jun 10, 2015

larsoner commented Jun 10, 2015

larsoner commented Jul 7, 2015

larsoner commented Jul 9, 2015

dengemann commented Jul 9, 2015

larsoner commented Jul 9, 2015

larsoner commented Jul 9, 2015

dengemann commented Jul 9, 2015

larsoner commented Jul 9, 2015

dengemann commented Jul 9, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

larsoner commented Jul 9, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

larsoner commented Jul 9, 2015

dengemann commented Jul 9, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

agramfort commented Jul 13, 2015

larsoner commented Jul 13, 2015

agramfort commented Jul 13, 2015 via email

larsoner commented Jul 13, 2015

dengemann commented Jul 13, 2015