facebookresearch / fastMRI

A large-scale dataset of both raw MRI measurements and clinical MRI images.
https://fastmri.org
MIT License
1.3k stars 372 forks source link

Error when running CS BART example #132

Closed gabrielziegler3 closed 3 years ago

gabrielziegler3 commented 3 years ago

I have been trying to run the cs example, but I keep getting the following error. I have looked upon BART's documentation and GitHub page, but I couldn't find much information about it. I am running this from inside a docker.

I am running in a docker with:

my command:

python3.8 ./fastMRI/fastmri_examples/cs/run_bart.py \
    --challenge "singlecoil" \
    --data_path "data/FastMRI/knee/" \
    --out_path "pred-recons" \
    --reg_wt 0.01 \
    --mask_type "random" \
    --split val

If I try the advised steps

Once BART is installed, set the TOOLBOX_PATH environment variable and point PYTHONPATH to the python wrapper for BART:

export TOOLBOX_PATH=$(which bart) # here I have tried a bunch of different options like /usr/local/bin, /usr/local/bin/bart, etc
export PYTHONPATH=${TOOLBOX_PATH}/python:${PYTHONPATH}

BART still can't be found via python even though I can call it from shell and I get:

Traceback (most recent call last):
  File "run_bart.py", line 15, in <module>
    import bart
ModuleNotFoundError: No module named 'bart'

My fix

I did manage to run the experiment by hardcoding the repository with sys.path.append inside run_bart.py

import sys
sys.path.append("./bart/python")      

What did I miss? I can probably fix this myself, but I am not aware of what exactly I missed.

EDIT: I forgot to mention that the correct argument for the script is out_path and not output_path as cited in the CS example README

mmuckley commented 3 years ago

Hello @gabrielziegler3, this looks like a path issue. The TOOLBOX_PATH should be where you have your BART directory, not a system directory like /usr/local/bin. In your case it looks like it's in the current working directory, so perhaps the output of the pwd command wherever you have run_bart.py.

gabrielziegler3 commented 3 years ago

Hi @mmuckley ! Hope you're well.

I thought it should point to where BART was installed in my OS. I have moved the BART directory up so Python wouldn't try to import the directory as the package, but instead, the python wrapper pointed by $PYTHONPATH.

My file structure is:

.
├── bart (clone of BART master 0.7.0)
├── compressed-sensing
│   ├── run_bart.py (extracted from fastmri_examples/cs)
│   └── run_bart.sh  (created by me to run from docker with python 3.8)
├── data
├── Dockerfile
├── fastMRI (clone of FastMRI master)
...
│   ├── fastmri
│   ├── fastmri_examples
...

My run_bart.sh from the compressed-sensing folder.

#!/bin/bash

TOOLBOX_PATH="/bart_demo/bart"
# PYTHONPATH=
PYTHONPATH=${TOOLBOX_PATH}/python:${PYTHONPATH}

echo $TOOLBOX_PATH
echo $PYTHONPATH

python3.8 run_bart.py \
    --challenge "singlecoil" \
    --data_path "../data/FastMRI/knee/" \
    --out_path "../pred-recons" \
    --reg_wt 0.01 \
    --mask_type "random" \
    --split val

my output:

/bart_demo/bart
/bart_demo/bart/python:
Imported bart from: <module 'bart' from '/bart_demo/bart/python/bart.py'>
sh: 1: /bart_demo/bart/bart: not found
sh: 1: /bart_demo/bart/bart: not found
sh: 1: /bart_demo/bart/bart: not found
sh: 1: /bart_demo/bart/bart: not found
sh: 1: /bart_demo/bart/bart: not found
sh: 1: /bart_demo/bart/bart: not found
sh: 1: /bart_demo/bart/bart: not found
sh: 1: /bart_demo/bart/bart: not found
sh: 1: /bart_demo/bart/bart: not found
sh: 1: /bart_demo/bart/bart: not found
sh: 1: /bart_demo/bart/bart: not found
sh: 1: /bart_demo/bart/bart: not found
multiprocessing.pool.RemoteTraceback: 
"""
Traceback (most recent call last):
  File "/usr/lib/python3.8/multiprocessing/pool.py", line 125, in worker
    result = (True, func(*args, **kwds))
  File "/usr/lib/python3.8/multiprocessing/pool.py", line 48, in mapstar
    return list(map(*args))
  File "run_bart.py", line 175, in run_model
    prediction = cs_total_variation(
  File "run_bart.py", line 128, in cs_total_variation
    sens_maps = bart.bart(1, "ecalib -d0 -m1", kspace)
  File "/bart_demo/bart/python/bart.py", line 83, in bart
    raise Exception("Command exited with an error.")
Exception: Command exited with an error.
"""

The above exception was the direct cause of the following exception:

Traceback (most recent call last):
  File "run_bart.py", line 280, in <module>
    run_bart(args)
  File "run_bart.py", line 193, in run_bart
    outputs = pool.map(run_model, range(len(dataset)))
  File "/usr/lib/python3.8/multiprocessing/pool.py", line 364, in map
    return self._map_async(func, iterable, mapstar, chunksize).get()
  File "/usr/lib/python3.8/multiprocessing/pool.py", line 768, in get
    raise self._value
Exception: Command exited with an error.

So the module was loaded from the correct location, but $PYTHONPATH apparently appended another "bart" there. So if I try to change env variables in run_bart.sh to:

TOOLBOX_PATH="/bart_demo"
PYTHONPATH=${TOOLBOX_PATH}/bart/python:${PYTHONPATH}

I get the following error (which I am not sure if it is fastMRI, BART or my fault).

/bart_demo
/bart_demo/bart/python:/bart_demo/python:
Imported bart from: <module 'bart' from '/bart_demo/bart/python/bart.py'>
sh: 1: /bart_demo/bart: Permission denied
sh: 1: /bart_demo/bart: Permission denied
sh: 1: /bart_demo/bart: Permission denied
sh: 1: /bart_demo/bart: Permission denied
sh: 1: /bart_demo/bart: Permission denied
sh: 1: /bart_demo/bart: Permission denied
sh: 1: /bart_demo/bart: Permission denied
sh: 1: /bart_demo/bart: Permission denied
sh: 1: /bart_demo/bart: Permission denied
sh: 1: /bart_demo/bart: Permission denied
sh: 1: /bart_demo/bart: Permission denied
sh: 1: /bart_demo/bart: Permission denied
multiprocessing.pool.RemoteTraceback: 
"""
Traceback (most recent call last):
  File "/usr/lib/python3.8/multiprocessing/pool.py", line 125, in worker
    result = (True, func(*args, **kwds))
  File "/usr/lib/python3.8/multiprocessing/pool.py", line 48, in mapstar
    return list(map(*args))
  File "run_bart.py", line 175, in run_model
    prediction = cs_total_variation(
  File "run_bart.py", line 128, in cs_total_variation
    sens_maps = bart.bart(1, "ecalib -d0 -m1", kspace)
  File "/bart_demo/bart/python/bart.py", line 83, in bart
    raise Exception("Command exited with an error.")
Exception: Command exited with an error.
"""

The above exception was the direct cause of the following exception:

Traceback (most recent call last):
  File "run_bart.py", line 280, in <module>
    run_bart(args)
  File "run_bart.py", line 193, in run_bart
    outputs = pool.map(run_model, range(len(dataset)))
  File "/usr/lib/python3.8/multiprocessing/pool.py", line 364, in map
    return self._map_async(func, iterable, mapstar, chunksize).get()
  File "/usr/lib/python3.8/multiprocessing/pool.py", line 768, in get
    raise self._value
Exception: Command exited with an error.

Also, output_path in the README should be replaced with out_path, which is the correct argument expected. I have already changed this in a fork with some comments in the README section regarding the path problem above, but was unsure if I should open another issue or if I could submit a PR tagging this issue.

mmuckley commented 3 years ago

Ah I see. In this case it is trying to access your BART files and you're getting a permission denied, possibly because they're not executable or because you put them in your system folder. You get a list of messages because the processing is parallelized and you have one error per process.

When I tested this code, I had something like where the bart clone was in my home directory (e.g., ~/bart). I compiled BART and set TOOLBOX_PATH to ~/bart. Then, I set PYTHONPATH to ${TOOLBOX_PATH}python:${PYTHONPATH}.

In this case I would recommend asking the BART people about installation and make sure you can import bart and run the basic bart.bart command on a single file or k-space inside your container. Once you have that figured out we can pick this up and see if there are remaining fastMRI issues.

Also, thanks for raising the flag about out_path. I think output_path is better - will try to look at this more later.

gabrielziegler3 commented 3 years ago

Alright! I'll give it a try later. I have submitted a PR changing to out_path, but if it makes more sense I can change Zero Filled and CS experiments so both use the standardised output_path, which I also think is better.

mmuckley commented 3 years ago

Yeah feel free to change both of them! I'll comment over there when I get a chance to look.

mmuckley commented 3 years ago

I didn't see any issues in PR #135 so I went ahead and merged it. If you find an issue feel free to open a new PR.

gabrielziegler3 commented 3 years ago

I'm now closing this issue as it seems more like it could be a problem on the BART side. Thanks for the help @mmuckley