CoBrALab / RABIES

fMRI preprocessing pipeline and analysis tools adapted for rodent images. Visit the full documentation at https://rabies.readthedocs.io/en/stable/
Other
34 stars 14 forks source link

--conf_list argument format unclear from -h #94

Closed grandjeanlab closed 4 years ago

grandjeanlab commented 4 years ago

The following wouldn't be recognized.

--conf_list [{WM_signal,CSF_signal,mot_6}] --conf_list [{'WM_signal,'CSF_signal','mot_6'}] --conf_list ['WM_signal,'CSF_signal','mot_6']

Please provide an example in readme or clarify in -h.

grandjeanlab commented 4 years ago

potentially, same for list of seeds.

grandjeanlab commented 4 years ago

I actually have issues running --seed_list in the 'analysis'. The following command does not register the confound_folder and output folder

rabies analysis --seed_list $ROI_DIR/S1bf_l.nii.gz $INPUT_DIR/ds01002 $OUTPUT_DIR/ds01002

Gab-D-G commented 4 years ago

The syntax is: --conf_list WM_signal,CSF_signal Same thing for seed_list, but with file names. I will add an example on the README I am not sure what you mean by registering the folders. Maybe there is an error because you are entering optional arguments (--seed_list) before providing the input/output folders, which have to be the first 2 arguments. Let me know if this is not the issue.

grandjeanlab commented 4 years ago

The following command with the conf list as specified returns the following:

command: rabies confound_regression --commonspace_bold --highpass 0.01 --lowpass 0.1 --smoothing_filter 0.5 --conf_list WM_signal,CSF_signal --diagnosis_output /project/4180000.19/multiRat/preprocess/ds01002 /project/4180000.19/multiRat/denoise/ds01002_wm

error: rabies confound_regression: error: argument --conf_list: invalid choice: 'WM_signal,CSF_signal' (choose from 'WM_signal', 'CSF_signal', 'vascular_signal', 'global_signal', 'aCompCor', 'mot_6', 'mot_24', 'mean_FD')

The alternative command with the arguments at the end return the same error: rabies confound_regression /project/4180000.19/multiRat/preprocess/ds01002 /project/4180000.19/multiRat/denoise/ds01002_wm --commonspace_bold --highpass 0.01 --lowpass 0.1 --smoothing_filter 0.5 --diagnosis_output --conf_list WM_signal,CSF_signal

grandjeanlab commented 4 years ago

for seed_list, putting the arguement at the end worked. This should also be clarified in the readme/help. Currently, it seems the optional arguments should be in between the sub-function specification (e.g. preprocessing, analysis), and the input/output directories.

grandjeanlab commented 4 years ago

also also, strangely, adding two seeds as in: rabies analysis /project/4180000.19/multiRat/denoise/ds01002 /project/4180000.19/multiRat/analysis/ds01002 --seed_list /project/4180000.19/multiRat/template/roi/S1bf_l.nii.gz,/project/4180000.19/multiRat/template/roi/S1bf_l2.nii.gz

causes an error during execution, but not when only one seed is specified. Seemingly, even though the process should be run in linear mode, it tries to do job submission with qsub, which causes the crash:

Traceback (most recent call last): File "/home/rabies/RABIES-0.2.0-dev/bin/rabies", line 3, in <module> execute_workflow() File "/home/rabies/RABIES-0.2.0-dev/rabies/run_main.py", line 322, in execute_workflow 'n_procs': opts.local_threads, 'qsub_args': '-pe smp %s' % (str(opts.min_proc))}) File "/home/rabies/miniconda-latest/envs/rabies/lib/python3.6/site-packages/nipype/pipeline/engine/workflows.py", line 595, in run runner.run(execgraph, updatehash=updatehash, config=self.config)

Gab-D-G commented 4 years ago

My appologies for the late response. For providing multiple arguments, I thought that separating them by a comma would work, but you should try instead a space in between: --conf_list WM_signal CSF_signal and --seed_list /project/4180000.19/multiRat/template/roi/S1bf_l.nii.gz /project/4180000.19/multiRat/template/roi/S1bf_l2.nii.gz I hope this works! Gabe

grandjeanlab commented 4 years ago

That worked for --conf_list, but not for --seed_list. In my hands, using two arguments for --seed_list, only the last argument gets taken into account.

I can work with that for now, but this might be looked into in the next commits. Also, as suggested, you might want to clarify README and -h