nipreps / fmriprep

fMRIPrep is a robust and easy-to-use pipeline for preprocessing of diverse fMRI data. The transparent workflow dispenses of manual intervention, thereby ensuring the reproducibility of the results.
https://fmriprep.org
Apache License 2.0
634 stars 293 forks source link

still demands FS license even with --fs-no-reconall #1747

Open yarikoptic opened 5 years ago

yarikoptic commented 5 years ago
$> singularity run containers/images/bids/bids-fmriprep--1.4.1.sing  sourcedata . participant --skip_bids_validation --no-freesurfer                          
Traceback (most recent call last):
  File "/usr/local/miniconda/bin/fmriprep", line 10, in <module>
    sys.exit(main())
  File "/usr/local/miniconda/lib/python3.7/site-packages/fmriprep/cli/run.py", line 326, in main
    (for free) by registering at https://surfer.nmr.mgh.harvard.edu/registration.html""")
RuntimeError: ERROR: a valid license file is required for FreeSurfer to run. fMRIPrep looked for an existing license file at several paths, in this order: 1) command line argument ``--fs-license-file``; 2) ``$FS_LICENSE`` environment variable; and 3) the ``$FREESURFER_HOME/license.txt`` path. Get it (for free) by registering at https://surfer.nmr.mgh.harvard.edu/registration.html
Sentry is attempting to send 0 pending error messages
effigies commented 5 years ago

Yes. There are other uses of Freesurfer tools. There are some invocations that avoid them, but we stopped trying to calculate whether it'd be needed.

emdupre commented 5 years ago

That's mentioned here, but maybe it could be clearer ? If you can think of a rephrasing (or another way to highlight this information), that would be great !

yarikoptic commented 5 years ago

so in summary:

as for a "fix", since --no-freesurfer is deprecated, I guess it is ok. Otherwise I would have just suggested to add "You will still need FreeSurfer license even if you do not use reconall" or alike

effigies commented 5 years ago

These are meant to be useful checks, not limitations on user freedom. Users were running into FreeSurfer license failures late in execution, which can be extremely annoying, and we would need to do a fairly complicated parse of the BIDS directory and options set to determine at initialization whether they would be run into.

If someone is confident they won't run into a FreeSurfer tool, we could allow them to override the check, similar to how we allow them to override the BIDS validation check. As an off-the-cuff proposal: switch from --skip-bids-validation to --skip-check and allow bids and fslicense as arguments.

An alternative could be a bit more complicated: build the workflow graph, and look for nipype.interfaces.freesurfer.base.FSCommands, and possibly a handful of other classes that don't inherit from that. This would be pretty expensive, but nothing compared to the actual runtime, and would be less susceptible to getting out of sync with the workflow than a big if-else tree.