search

pnlbwh / CNN-Diffusion-MRIBrain-Segmentation

CNN based brain masking
Other
14 stars 10 forks source link

Remove external dependencies #7

Closed tashrifbillah closed 4 years ago

tashrifbillah commented 4 years ago

Hi @SenthilCaesar , please install two additional requirements and test my branch to make sure it is seamless with rest of your pipeline.

conversion and pnlNipype are already part of /path/to/pnlpipe3/bashrc3. If you source that, they are available to you automatically.

However, if you have a separate system for running this pipeline:

(i) you have to install https://github.com/pnlbwh/conversion using pip (see requirements.txt) (ii) on the other hand, I trust you to figure out dependencies of 'just' https://github.com/pnlbwh/pnlNipype/blob/master/scripts/bse.py so you can set up a separate pnlNipype of your own and run 'just' bse.py.

Once you find the modification seamless, we can merge this PR.

senthilcaesar commented 4 years ago

Hi Tashrif, I have tested the pipeline and the modification works fine . I think we can merge to the master branch.

tashrifbillah commented 4 years ago

Hi @SenthilCaesar , please update the README with installation and running instruction. To be more specific:

The goal is to allow me (or a remote user) run this software by their own. Check each box once done.

tashrifbillah commented 4 years ago

Hi @SenthilCaesar , does this software run without GPU support? If it does run on just CPU, what packages should we ask the user to install? https://github.com/pnlbwh/CNN-Diffusion-MRIBrain-Segmentation/tree/remove-ext-dep#install-prerequisites-for-running-the-pipeline

senthilcaesar commented 4 years ago

Hi @SenthilCaesar , does this software run without GPU support? If it does run on just CPU, what packages should we ask the user to install? https://github.com/pnlbwh/CNN-Diffusion-MRIBrain-Segmentation/tree/remove-ext-dep#install-prerequisites-for-running-the-pipeline

Yes, the software runs on CPU . The user should install pip install tensorflow ( this is CPU version ) / pip install tensorflow-gpu ( this is GPU version )

tashrifbillah commented 4 years ago

Did you get a chance to test the software installation and execution on a workstation using just CPU support? For example, we can install a fresh python3 in /tmp/miniconda3 and then follow your README.md to install and execute it.

senthilcaesar commented 4 years ago

Did you get a chance to test the software installation and execution on a workstation using just CPU support? For example, we can install a fresh python3 in /tmp/miniconda3 and then follow your README.md to install and execute it.

Yes, the pipeline was tested on Sylvain workstation CPU.

tashrifbillah commented 4 years ago

Hi @SenthilCaesar , please come back to remove-ext-dep branch as follows:

cd CNN-Diffusion-MRIBrain-Segmentation
cp src/settings.ini src/settings.senthil # back up your config file
git pull origin
git checkout remove-ext-dep
cp src/settings.senthil src/settings.ini

We shall do one more round of testing to find potential runtime errors. In the meantime, I would like you to review docs/README.md#training section. Once we are done, we shall release an affluent master branch.

tashrifbillah commented 4 years ago
tashrifbillah commented 4 years ago

@SenthilCaesar , please repeat the above tests on your GPU environment and comment like I did so we can keep track of testing.

Edit I am done with the above tests. Once you are done, we can merge remove-ext-dep into master. See below a few more commits I had to make. Please pull them first.

senthilcaesar commented 4 years ago

pipeline/dwi_masking.py works fine on GPU src/train.py work fine on GPU registration.py, preprocess_b0.py, preprocess_mask.py, and maskfilter.py help messages print properly on CPU without any TensorFlow warning