shimming-toolbox / shimming-toolbox-matlab

Code for performing real-time shimming using external MRI shim coils
GNU General Public License v3.0
16 stars 5 forks source link

Considering moving to Python #158

Open jcohenadad opened 4 years ago

jcohenadad commented 4 years ago

Pros:

Cons:

gab-berest commented 4 years ago

Pros: Community is a lot more active for Python than Matlab for every aspect, not just the neuroimaging community. Really complete unit test libraries and dummy libraries in Python.

kousu commented 4 years ago

https://numpy.org/devdocs/user/numpy-for-matlab-users.html helps move from matlab to python for scientific computation.

kousu commented 4 years ago

Optimization algorithms in python:

mathieuboudreau commented 4 years ago

It really isn't that difficult of a transition (MATLAB->Python, not rewriting code) in my opinion - you just need to know/find which tools have the functions that you're used to using in MATLAB. Also know that everything in Python is a class object "underneath the hood". The biggest hurdle is if any of your features depend on a tool that was written for MATLAB - rewriting that tool is likely out of the question, so you either have to find a replacement or give up on that feature.

And once you switch from MATLAB to Python, the added features that @jcohenadad listed and many more (e.g. a very active open-source community, jupyter notebooks, binder, plotly, etc.) will make your life so much easier that you might never want to turn back...

one-of-us-1-one-of-us-34138353

jcohenadad commented 4 years ago

Strategy is to:

jcohenadad commented 4 years ago

New repos: https://github.com/shimming-toolbox/shimming-toolbox-py

kousu commented 4 years ago

For an example, to port the b0 map step, I would start by making this CLI:

./b0maps [--coilshims arduino_config.txt] [--pulseseq pulse_calibration.txt] dicom_folder/ output_fieldmap.nii

For some starter code, you can use argparse, but I am a real big fan of click, so that's the sketch I'm going to write now:

#!/usr/bin/env python3

import click

import logging

@click.command()
@click.option("--verbose", is_flag=True, help="Be more verbose.")
@click.option("--coilshims", help="Output hardware-shim calibration.")
@click.option("--pulseseq", help="Output pulse-sequence calibration.")
@click.argument("input")
@click.argument("output")
def cli(verbose, coilshims, pulseseq, input, output):
    """
    Compute b0 fieldmap OUTPUT from a folder of dicom files INPUT.
    """
    if verbose:
        logging.getLogger().setLevel(logging.INFO)
    logging.info(f'{coilshims}, {pulseseq}, {input}, {output}')
    raise NotImplementedError()

if __name__ == '__main__':
    cli()

On mac/linux you can put that into "b0maps" and then run it like:

$ chmod +x b0maps
$ ./b0maps --verbose dcms map.nii.gz
INFO:root:None, None, dcms, map.nii.gz
Traceback (most recent call last):
  File "./b0maps", line 23, in <module>
    cli()
  File "/home/kousu/.local/lib/python3.6/site-packages/click/core.py", line 829, in __call__
    return self.main(*args, **kwargs)
  File "/home/kousu/.local/lib/python3.6/site-packages/click/core.py", line 782, in main
    rv = self.invoke(ctx)
  File "/home/kousu/.local/lib/python3.6/site-packages/click/core.py", line 1066, in invoke
    return ctx.invoke(self.callback, **ctx.params)
  File "/home/kousu/.local/lib/python3.6/site-packages/click/core.py", line 610, in invoke
    return callback(*args, **kwargs)
  File "./b0maps", line 20, in cli
    raise NotImplementedError()
NotImplementedError

On Windows the first line, the "shebang", doesn't work, but you can save it to b0maps.py instead and do

C:\...\> python b0maps.py --verbose dcms map.nii.gz
jcohenadad commented 4 years ago

thank you for the head start here https://github.com/shimming-toolbox/shimming-toolbox/issues/158#issuecomment-653669780 @kousu.

however, i would suggest to separate CLIs as much as possible, i.e. not mix dcm2nii conversion and b0map fitting (because we know that later one these will have to be separated anyway)

how about following the latest examples we did? @gaspardcereza @po09i are working on example scenario scripts, so maybe we can follow this sequence of calls instead. also, currently, those example scripts are in gdoc, which is not visible for everyone, and is also confusing for newcomers to this project. i would suggest to:

does that make sense?

gaspardcereza commented 4 years ago

Right now we don't have all the functions required to run a full scenario (from dicoms to shim calculation) so that's why we decided to go with a gdoc instead of a code that wouldn't be fully functional. Would it be ok if the parts of the code that are not available yet were just commented until we create them ? That would also allow us to easily keep track of what has been done so far and what remains to be done.

jcohenadad commented 4 years ago

Right now we don't have all the functions required to run a full scenario (from dicoms to shim calculation) so that's why we decided to go with a gdoc instead of a code that wouldn't be fully functional. Would it be ok if the parts of the code that are not available yet were just commented until we create them ?

definitely ok! the goal here is to get started immediately with something

That would also allow us to easily keep track of what has been done so far and what remains to be done.

yup!

again, i would strongly encourage to centralize everything in github. This is much easier to manage the project.

jcohenadad commented 4 years ago

this is happening 🙌 --> https://github.com/shimming-toolbox/shimming-toolbox-py