openmeeg / openmeeg

A C++ package for low-frequency bio-electromagnetism solving forward problems in the field of EEG and MEG.
http://openmeeg.github.io
Other
74 stars 51 forks source link

Rewrite apps in python (keeping the core library in C++) #440

Open papadop opened 2 years ago

papadop commented 2 years ago

Advantages:

Drawbacks:

agramfort commented 2 years ago

@ftadel what do you think? would it be a deal breaker for many brainstorm users?

ftadel commented 2 years ago

We've done all the work to call Python functions from Brainstorm using the Matlab-Python integration. This way we can call functions from MNE-Python or other libraries:

However, setting up a Python environment is probably too complicated or time-consuming for many of the Brainstorm users working with Windows. In clinical contexts (100% Windows, and sometimes still Win7), it's sometimes difficult for people to use tools that are not fully graphical and plug-and-play.

I understand why it would be technically better for OpenMEEG to be interfaced in Python. But if you consider that a good share of OpenMEEG users are coming from Brainstorm, then this might not be the most appropriate choice, at least not until Python is available natively on all the OS.

If the future versions of OpenMEEG are Python-based only, I guess we'd have to stick to the old compiled versions for Brainstorm, at least for the standard users. For advanced users, we could write a Python wrapper to access the new version (I hope you'd help me write, document and maintain this code).

The most versatile solution would be: a C++ library + a Python wrapper + a Matlab wrapper... (all the arguments listed above are also valid for Matlab)

Possible interfacing options:

agramfort commented 2 years ago

I had the same feeling here. Python running in matlab is very much a niche setup and I doubt it will keep the user base of openmeeg stable in the matlab/brainstorm ecosystem. We need a Python wrapper in openmeeg but it should not become the only way to do things.

my 2c

Message ID: @.***>

vhs304305 commented 2 years ago

Please spend time to fix the existing bugs and improve the documentation first. That would most likely increase and stabilize the user base.

papadop commented 2 years ago

@vhs304305: Which bugs are you referring to ? If you have specific bugs or documentation improvement suggestions, please feel issues....

papadop commented 2 years ago

Just for the record, I did a simple test with the simplest app: minverser. I have not fully tested it yet (I'm unsure of the type=argparse.FileType('w')), but with -h, it provides a nice output, and without the inner code, it gives a nice output.... Copy/pasting the code (replacing args with actual file names) seems to write a correct file).

This gives:

import sys
import time
import argparse
import openmeeg as om

parser = argparse.ArgumentParser(description='Compute the inverse of an head matrix.')
parser.add_argument('source_file', metavar='head_matrix', type=open, nargs=1, help='file name of the head matrix')
parser.add_argument('dest_file', metavar='inverse_head_matrix', type=argparse.FileType('w'), nargs=1, help='file name to store the inverse head matrix')

args = parser.parse_args()
print('-------------------------------------------')
print('Command line:')
print(*sys.argv)
print('-------------------------------------------')

start = time.time()

head_matrix = om.SymMatrix()
head_matrix.load(args.source_file)
head_matrix.invert()
head_matrix.save(args.dest_file)

end = time.time()

print('-------------------------------------------')
print('Elapsed Time: ', end - start, ' s.')
print('-------------------------------------------')