poldracklab / fitlins Goto Github PK

#30 dropped support for 3+ level models. To fix this, it will be easiest to find an example model to support.

This will make building neuroscout images with the latest FitLins changes trivial.

It would be good to get a test that actually constructs and runs a workflow into the CI tests. We could then start catching breaking changes coming down the pipeline from pybids and nistats.

@adelavega Maybe a small neuroscout model and a pre-packaged input that we can throw into OSF (or pull down off DataLad) would be good? We could just run the FitLins command directly, rather than going through neuroscout as an intermediary.

If the current level is 'dataset', and there are multiple runs for a single subject, this is probably a specification error. Nistats currently can't handle random effects specification, so there would be no way to indicate that subject is meant to be a random effect (which is almost certainly what users will want). From a BIDS-StatsModel perspective, the solution is to insert a 'subject' step in between 'run' and 'dataset', which implicitly averages within-subject and propagates only a single estimate per subject to the dataset level.

I don't think we want to raise an exception, as there could be use cases where one really does want to treat subject as fixed, but by default there should be a prominent warning somewhere. (This could potentially come at load-time right after the get_design_matrix call to pybids—and in fact, could even be in pybids itself).

applying snake_to_camel to ents in BIDSDataSink will fail on "run", since it is an integer, not a string:

File "/src/fitlins/fitlins/interfaces/bids.py", line 361, in <dictcomp>
ents = {k: snake_to_camel(v) for k, v in ents.items()}  
File "/src/fitlins/fitlins/utils/strings.py", line 2, in snake_to_camel    
words = string.split('_')
AttributeError: 'numpy.int64' object has no attribute 'split'       

  File "/opt/conda/envs/neuro/lib/python3.6/site-packages/nipype/pipeline/plugins/multiproc.py", line 69, in run_node
    result['result'] = node.run(updatehash=updatehash)
  File "/opt/conda/envs/neuro/lib/python3.6/site-packages/nipype/pipeline/engine/nodes.py", line 471, in run
    result = self._run_interface(execute=True)
  File "/opt/conda/envs/neuro/lib/python3.6/site-packages/nipype/pipeline/engine/nodes.py", line 555, in _run_interface
    return self._run_command(execute)
  File "/opt/conda/envs/neuro/lib/python3.6/site-packages/nipype/pipeline/engine/nodes.py", line 635, in _run_command
    result = self._interface.run(cwd=outdir)
  File "/opt/conda/envs/neuro/lib/python3.6/site-packages/nipype/interfaces/base/core.py", line 521, in run
    runtime = self._run_interface(runtime)
  File "/src/fitlins/fitlins/interfaces/nistats.py", line 101, in _run_interface
    confounds = pd.read_hdf(info['confounds'], key='confounds')
  File "/opt/conda/envs/neuro/lib/python3.6/site-packages/pandas/io/pytables.py", line 371, in read_hdf
    'File %s does not exist' % path_or_buf)
FileNotFoundError: File None does not exist

I'm looking into it

This would be a good source of test cases. It would also make sense to focus on datasets that are already preprocessed on OpenNeuro.

When loading BIDS metadata, indices (for example: run indices) are converted to integers, and thus lose zero-pad information.

For mostly aesthetic reasons, it would be good to use the same zero-padding as found in the original files.

It may be useful to output the final design matrix file (in addition to the design matrix plot in the report)

Often, if the analysis crashed prior to any ouputs being generated, a folder with just a dataset_description.json is made. It'd be good to avoid making an output folder if the analysis crashes at an early enough stage.

Minor, but blue = positive, red=negative in fitlins brain plots.

I'm guessing you know about this, but I still thought I'd make an issue so at least you can let me know how you are testing/debugging.

Exceptions seem to be captured in a way that they do not propagate down to the user. It's a bit confusing, because its not clear what went wrong. Running in IPython, I'm able to get to the tracebacks, so its not an urgent issue for me.

For example:

docker run -v D:\datasets\:/datasets  -v E:\analyses\:/analysis fitlins  
/datasets/data /analysis/out dataset --model /analysis/does_not_exist.json

The following fails because the mode file does not exist. However, running this command the full output I get is:

/opt/conda/envs/neuro/lib/python3.6/site-packages/h5py/__init__.py:36: FutureWarning: Conversion of            the second argument of issubdtype from `float` to `np.floating` is deprecated. In future, it will be treated as `np.float64 == np.dtype(float).type`. 
from ._conv import register_converters as _register_converter
Captured warning (<class 'UserWarning'>): No valid root directory found for domain 'derivatives'. Falling back on the Layout's root directory. If this isn't the intended behavior, make sure the config file for this domain includes a 'root' key.

When debugging in Ipython, I can see the relevant traceback:

*** FileNotFoundError: [Errno 2] No such file or directory: '/analysis/does_not_exist.json'

Ever since I started to use fitlins I've been confused why this required analysis_level argument is (still) in existence:

I believe it has been deprecated half a year ago, and I can't find it being used anywhere, but running fitlins without it leads to

usage: fitlins [-h] [-v]
               [--participant-label PARTICIPANT_LABEL [PARTICIPANT_LABEL ...]]
               [-m MODEL] [-d DERIVATIVES [DERIVATIVES ...]]
               [--derivative-label DERIVATIVE_LABEL]
               [--space {MNI152NLin2009cAsym,,MNI152NLin6Sym}]
               [--include INCLUDE] [--exclude EXCLUDE] [--desc DESC]
               [--n-cpus N_CPUS] [--debug] [-w WORK_DIR]
               bids_dir output_dir {run,session,participant,dataset}
fitlins: error: the following arguments are required: analysis_level

I could try to contribute and make analysis_level a 'stopping criterion' to halt the computations once the specified analysis level is reached, if that is intended/useful functionality. Otherwise I could just remove it from argparse.

Currently, it seems fitlins only outputs stat maps. Outputting beta maps would be very useful.

Within a level, each fit/contrast step is independent, making this a good target for parallelization.

This thread is a place to make notes on ideas and steps taken before we get to the stage of opening a PR.

@satra @tyarkoni @adelavega We did a bit of strategizing on this in Austin, but it's taken a bit of a back burner until now. Feel free to add any points from those discussions you think important for directing this, or any ideas you may have had since.

Fitlins assumes that there will be confounds specified which is not guaranteed. If no confounds are present (i.e. variables in 'variables' but not 'HRF_variables'), an error occurs:

  File "/opt/conda/envs/neuro/lib/python3.6/site-packages/nipype/pipeline/plugins/multiproc.py", line 68, in run_node
    result['result'] = node.run(updatehash=updatehash)
  File "/opt/conda/envs/neuro/lib/python3.6/site-packages/nipype/pipeline/engine/nodes.py", line 480, in run
    result = self._run_interface(execute=True)
  File "/opt/conda/envs/neuro/lib/python3.6/site-packages/nipype/pipeline/engine/nodes.py", line 564, in _run_interface
    return self._run_command(execute)
  File "/opt/conda/envs/neuro/lib/python3.6/site-packages/nipype/pipeline/engine/nodes.py", line 644, in _run_command
    result = self._interface.run(cwd=outdir)
  File "/opt/conda/envs/neuro/lib/python3.6/site-packages/nipype/interfaces/base/core.py", line 520, in run
    runtime = self._run_interface(runtime)
  File "/work/src/fitlins/fitlins/interfaces/bids.py", line 191, in _run_interface
    names = [col for col in confounds.columns
AttributeError: 'NoneType' object has no attribute 'columns'

Seems like an easy enough fix though.

When creating the group level svg plot the following error occurs:

FileNotFoundError: [Errno 2] No such file or directory: '/analysis/out_all2/fitlins/ses-
perceptionTraining01/ses-perceptionTraining01_task-perception_run-1_bold_contrasts.svg'

Looks like the parent folder is never created prior to outputting the SVG, hence the error.

Related: #100

Hi Everyone,

Now that Derivatives (BEP003) and Linear Models (BEP002) are getting into somewhat usable shape, I wonder what are the plans/schedule for the fitlins -- will it be developed soon to accommodate for all the changes in those BEPs or there is another module/framework in works to look at in attempt to make initial use of BEP002?

Cheers!

This will probably require someone with extensive FSL experience, as the nipype Level1Design is very convoluted.

This issue may no longer be relevant (yet to test on new version), but thought I'd file it for permanence.

I'm having an issue where the header of an fMRIprep preproc file indicates the TR is 0.8, but it is read in as a float32. This causes a rounding error when calculating the sampling rate:

1 / TR = 1.249999999

This causes downstream problems in pybids. For now, I'm converting to a regular float and rounding to 5 decimals, but it might be worth making fitlins robust to such floating point imprecisions.

Occasionally, confounds have missing values

For example, for SherlockMerlin sub-28 MerlinMovie task, the first volume for FrameWiseDisplacement is n/a.

Regardless of the reason, we should be prepared to deal with such values.

Currently, the n/a value will be skipped (by pybids), and result in an incorrect number of volumes in the confounds file passed to nistats. Resulting in this error:

  File "/opt/conda/envs/neuro/lib/python3.6/site-packages/nistats/design_matrix.py", line 351, in make_design_matrix
    'time-frames: %d' % (add_regs.shape[0], np.size(frame_times)))
AssertionError: Incorrect specification of additional regressors: length of regressors provided: 1026, number of time-frames: 1027

The "proper" BIDS model solution would be the replace n/as with the mean value using a transformation, but that seems too cumbersome. Probably best if fitlins does this if any n/as are detected in confounds.

Really minor, but opts.preproc_dir should be preproc_dir, since that variable is processed to have the correct directory, rather than just fmriprep

Following up on #91, we should figure out how to accommodate:

• 0 or 1 raw datasets
• 0 or more derivatives datasets

With a consistent API.

Since raw datasets are expected by BIDS-Apps, I would propose the special value 'none' for 0 raw datasets. An alternative would be some way to specify that the "base" dataset should be parsed as a derivatives dataset, but given that we have the -d flag, I think it's a little simpler to keep it as raw.

@yarikoptic mentioned we should consider the case where no derivatives datasets should be parsed, which means there's a distinction between <default> and 0 derivatives.

background: Together with @AdinaWagner we decided to dig ourselves into a rabbit hole and try to get fitlins work on a our sample (well, forrest gump) preprocessed (aligned according to BIDS Derivatives) dataset. Some results have appeared as pull requests against PyBIDS which we took of the bleeding edge version. We "dumbed down" our model.json from BEP002 to fitlins format (since support of BEP002 is just planned). And now we are stuck at the point of https://github.com/poldracklab/fitlins/blob/master/fitlins/interfaces/bids.py#L276 . pybids has changed output of get_contrasts and I thought to see how "original" fitlins+pybids worked on fitlins models, and thought to run the tests to get to that point, but as I discovered -- there seems to be no single unittest or working example!
We have tried to run the beast on the sample ds000003 model, while forcing it to digest raw bids dataset:

$> fitlins ~/datalad/openfmri/ds000003/ output-ds0000003 run --participant-label 01 -m $PWD/fitlins-models/openfmri/ds000003/model.json -p ~/datalad/openfmri/ds000003/  
...
  File "/home/yoh/proj/bids/try-fitlins/venvs/dev/lib/python3.6/site-packages/bids/analysis/analysis.py", line 89, in setup
    b.setup(input_nodes, auto_contrasts, **selectors)
  File "/home/yoh/proj/bids/try-fitlins/venvs/dev/lib/python3.6/site-packages/bids/analysis/analysis.py", line 189, in setup
    collections = self.layout.get_collections(self.level, **kwargs)
  File "/home/yoh/proj/bids/try-fitlins/venvs/dev/lib/python3.6/site-packages/bids/layout/bids_layout.py", line 317, in get_collections
    skip_empty=skip_empty, **kwargs)
  File "/home/yoh/proj/bids/try-fitlins/venvs/dev/lib/python3.6/site-packages/bids/variables/io.py", line 73, in load_variables
    dataset = _load_time_variables(layout, dataset, **_kwargs)
  File "/home/yoh/proj/bids/try-fitlins/venvs/dev/lib/python3.6/site-packages/bids/variables/io.py", line 141, in _load_time_variables
    raise ValueError("No functional images that match criteria found.")
ValueError: No functional images that match criteria found.

which, from our "tour" through fitlins I think is due to looking for

(git)hopa:~/proj/bids/try-fitlins/fitlins[master]git
$> git grep "'preproc" 
fitlins/interfaces/bids.py:            space = analysis.layout.get_spaces(type='preproc',
fitlins/interfaces/bids.py:            preproc_files = analysis.layout.get(type='preproc',
fitlins/workflows/base.py:                   selectors={'type': 'preproc', 'space': space}),

Is there an easy to try out example setup (model/dataset/command), even if it would be a toy one?

Adding an option to delegate isotropic smoothing to Nistats by specifying FWHM in the model.json file.

In a dataset w/ 4 runs per task, and only one run selected in the model (run: 1), no run entity was recorded in the contrast files output by BIDSDataSink. The second level then can't find the files, since it's using run as en entity to .get files with.

Would you be open to add a CLI flag to set bids.configs settings, such as loop_preproc=True?

This would be nice because currently I import fitlins and set bids.configs, prior to calling run_fitlins. But we are trying to move towards just making a fully self contained fitlins call (either as a Docker container or a pip installed version). This makes it more reproducible, because all inputs and command arguments to fitlins are tracked.

What do you think?

Smoothing may be a BIDS-model transform. Thoughts @tyarkoni? Or is that separate from the statistical considerations, and thus a runtime knob to turn?

As of 0.7, pybids will support HRF convolution as a built-in transformation (and the BIDS-Model spec will soon be updated accordingly). This means we should be able to avoid HRF convolution in fitlins/nistats by passing the to-be-convolved variables to the convolve_HRF transformation. See here for usage details.

On Windows 10, when fitlins outputs to a mounted volume, there are occasional problems writing the output contrast files. E.g:

Error: Traceback (most recent call last):

  File "/opt/conda/envs/neuro/lib/python3.6/site-packages/nipype/pipeline/engine/utils.py", line 106, in nodelist_runner
    result = node.run(updatehash=updatehash)

  File "/opt/conda/envs/neuro/lib/python3.6/site-packages/nipype/pipeline/engine/nodes.py", line 480, in run
    result = self._run_interface(execute=True)

  File "/opt/conda/envs/neuro/lib/python3.6/site-packages/nipype/pipeline/engine/nodes.py", line 564, in _run_interface
    return self._run_command(execute)

  File "/opt/conda/envs/neuro/lib/python3.6/site-packages/nipype/pipeline/engine/nodes.py", line 644, in _run_command
    result = self._interface.run(cwd=outdir)

  File "/opt/conda/envs/neuro/lib/python3.6/site-packages/nipype/interfaces/base/core.py", line 521, in run
    outputs = self.aggregate_outputs(runtime)

  File "/opt/conda/envs/neuro/lib/python3.6/site-packages/nipype/interfaces/base/core.py", line 595, in aggregate_outputs
    predicted_outputs = self._list_outputs()

  File "/src/fitlins/fitlins/interfaces/bids.py", line 367, in _list_outputs
    _copy_or_convert(in_file, out_fname)

  File "/src/fitlins/fitlins/interfaces/bids.py", line 304, in _copy_or_convert
    copyfile(in_file, out_file, copy=True, use_hardlink=True)

  File "/opt/conda/envs/neuro/lib/python3.6/site-packages/nipype/utils/filemanip.py", line 487, in copyfile
    copy_related_files=False)

  File "/opt/conda/envs/neuro/lib/python3.6/site-packages/nipype/utils/filemanip.py", line 471, in copyfile
    shutil.copyfile(originalfile, newfile)

  File "/opt/conda/envs/neuro/lib/python3.6/shutil.py", line 121, in copyfile
    with open(dst, 'wb') as fdst:

FileNotFoundError: [Errno 2] No such file or directory: '/datasets/SherlockMerlin/derivatives/neuroscout/qW/fitlins/sub-34/sub-34_task-MerlinMovie_bold_space-MNI152NLin2009cAsym_contrast-rmse_stat.nii.gz'

This problem goes away when outputting within the fitlins image.

See #111.

To add to the pile of Windows specific errors, when running fitlins reading from a datalad directory (installed on a mounted volume, from within the container) with n-cpu > 1, the following error occurs on prepoc and brainmask files (seems random when it occurs):

traits.trait_errors.TraitError: The trait 'mask_file' of a FirstLevelModelInputSpec instance is an existing file name, but the path  '/data/Life/derivatives/fmriprep/fm
riprep/sub-rid000017/func/sub-rid000017_task-life_acq-374vol_run-01_bold_space-MNI152NLin2009cAsym_brainmask.nii.gz' does not exist.
ERROR:concurrent.futures:exception calling callback for <Future at 0x7fada756c5f8 state=finished raised BrokenProcessPool>

Not really sure what to do/say about it, except that it's a known issue, and its more the reason to get a Linux box for running analyses. On my Ubuntu laptop, this is not an issue. I'm guessing it's related to how Windows handles symlinks.

It will help explain what fitlins is doing.

It would be good to get a sense of how many users we have and their issues.

We should perhaps try a different tack to fMRIPrep's opt-out approach. Not sure what the most container-friendly way to do that is. Perhaps an active question in the absence of a config file, with images that are pre-set for opt-in and opt-out, allowing the choice of container to serve for consent/withholding.

To make it easier for me (or others) to use fitlins as a "library", it would be useful to abstract out logic specific to the CLI.

However, I know its not the intention for fitlins to be generally used a library, so I propose some minimalist changes. Currently the only issue I'm having relates to the sys.exit and retcode. Since fitlins forces a sys.exit this will also exit Neuroscout, and prevent me from doing any other operations following execution. An easy fix would be to add a run function that does what main does now (parse args, run create_workflow, etc), but simply returns the retcode. main would then call run and execute sys.exit. I would then import run instead of `main.

Alternatively, parse args logic could be moved to create_workflow and sys.exit could be called in main, and I would import create_workflow.

Automatically-generated models were introduced into pybids with bids-standard/pybids#119. In the absence of a model, we should run an automatically generated model.

I'm getting the following error when running fitlins:

ERROR:nipype.workflow:Node _l1_model61 failed to run on host 99c5798a0bdf.
190207-17:51:32,776 nipype.workflow ERROR:
	 Saving crash info to /data/crash-20190207-175132-root-_l1_model61-45897725-e035-4beb-9f98-2757b4f391d0.pklz
Traceback (most recent call last):
  File "/opt/conda/envs/neuro/lib/python3.6/site-packages/nipype/pipeline/plugins/multiproc.py", line 69, in run_node
    result['result'] = node.run(updatehash=updatehash)
  File "/opt/conda/envs/neuro/lib/python3.6/site-packages/nipype/pipeline/engine/nodes.py", line 473, in run
    result = self._run_interface(execute=True)
  File "/opt/conda/envs/neuro/lib/python3.6/site-packages/nipype/pipeline/engine/nodes.py", line 557, in _run_interface
    return self._run_command(execute)
  File "/opt/conda/envs/neuro/lib/python3.6/site-packages/nipype/pipeline/engine/nodes.py", line 637, in _run_command
    result = self._interface.run(cwd=outdir)
  File "/opt/conda/envs/neuro/lib/python3.6/site-packages/nipype/interfaces/base/core.py", line 369, in run
    runtime = self._run_interface(runtime)
  File "/src/fitlins/fitlins/interfaces/nistats.py", line 62, in _run_interface
    sparse = pd.read_hdf(info['sparse'], key='sparse').rename(
  File "/opt/conda/envs/neuro/lib/python3.6/site-packages/pandas/io/pytables.py", line 394, in read_hdf
    return store.select(key, auto_close=auto_close, **kwargs)
  File "/opt/conda/envs/neuro/lib/python3.6/site-packages/pandas/io/pytables.py", line 741, in select
    return it.get_result()
  File "/opt/conda/envs/neuro/lib/python3.6/site-packages/pandas/io/pytables.py", line 1483, in get_result
    results = self.func(self.start, self.stop, where)
  File "/opt/conda/envs/neuro/lib/python3.6/site-packages/pandas/io/pytables.py", line 734, in func
    columns=columns)
  File "/opt/conda/envs/neuro/lib/python3.6/site-packages/pandas/io/pytables.py", line 2937, in read
    start=_start, stop=_stop)
  File "/opt/conda/envs/neuro/lib/python3.6/site-packages/pandas/io/pytables.py", line 2489, in read_array
    ret = node[0][start:stop]
  File "/opt/conda/envs/neuro/lib/python3.6/site-packages/tables/vlarray.py", line 671, in __getitem__
    return self.read(start, stop, step)[0]
  File "/opt/conda/envs/neuro/lib/python3.6/site-packages/tables/vlarray.py", line 811, in read
    listarr = self._read_array(start, stop, step)
  File "tables/hdf5extension.pyx", line 2106, in tables.hdf5extension.VLArray._read_array
ValueError: cannot set WRITEABLE flag to True of this array

It looks like it might be related to nipype, but it's not clear.

Given the BIDS App CLI:

$ app <bids> <out> <level> [<option> ...]

We should settle on a default location to look for the preprocessed dataset. In the case of:

$ fitlins /data/bids/ds000XYZ /data/bids/ds000xyz/derivatives <level> [...]

The reasonable default is /data/bids/ds000xyz/derivatives/fmriprep, but this is derivatives/fmriprep, relative to the BIDS root, and fmriprep relative to the output directory. When we have:

$ fitlins /data/bids/ds000XYZ /data/out/ds000xyz/derivatives <level> [...]

The options become:

• /data/bids/ds000xyz/derivatives/fmriprep
• /data/out/ds000xyz/derivatives/fmriprep

Up until now I have assumed the latter, reasoning that derivatives will tend to be kept together, even when not falling under the BIDS root directory. However, in #28, the proposed presumption is that preprocessed derivatives are special, and will be kept in the BIDS root directory even when the Fitlins derivatives to be written will go elsewhere.

It would be nice to get some intuitions from others. (I'll go ahead and tag @tyarkoni, @satra, @mgxd, @jordandekraker.)

To preempt what seems like an inevitable suggestion, I'm hesitant to make this too smart, where we first check for one and then the other, as adding the higher priority one to a setup that was using the lower-priority would silently change the behavior (assuming that the preprocessed files are different). A case to consider here is if the higher-priority one was misspelled (e.g. frmiprep), and we will silently fall back to the lower-priority one, correctly spelled directory. But maybe that's okay?

Finally, just as a UX note, the current behavior allows relative paths as well as absolute. So the actual default is -p fmriprep, which is resolved relative to out_dir. The idea here is to allow alternatives like -p spm or -p feat without requiring users to add a full path. So making the right default will affect ease-of-use in that case.

Currently images are passed from first_level() and to/from second_level() as a dictionary of files labeled by {variable_name: [files in order of block.get_design_matrix()]}.

To reorganize the files to fit a transformation, the following code is used (only split currently handled):

https://github.com/effigies/fitlins/blob/1e72b491884bd6719d02f53f23ebbd0599c73b85/fitlins/base.py#L98-L114

block.get_contrasts() is supposed to return an "identity matrix" that manages the ordering of files, which should be directly applicable to reorganizing files. At present, this looks something like:

                         finger_vs_others  finger_vs_others.retest  \
trial_type.Lips                       0.0                      0.0   
finger_vs_others.test                 0.0                      0.0   
trial_type.Finger                     0.0                      0.0   
finger_vs_others                      1.0                      0.0   
finger_vs_others.retest               0.0                      1.0   
lips_vs_others                        0.0                      0.0   
trial_type.Foot                       0.0                      0.0   

                         finger_vs_others.test  lips_vs_others  \
trial_type.Lips                            0.0             0.0   
finger_vs_others.test                      1.0             0.0   
trial_type.Finger                          0.0             0.0   
finger_vs_others                           0.0             0.0   
finger_vs_others.retest                    0.0             0.0   
lips_vs_others                             0.0             1.0   
trial_type.Foot                            0.0             0.0   

                         trial_type.Finger  trial_type.Foot  trial_type.Lips  
trial_type.Lips                        0.0              0.0              1.0  
finger_vs_others.test                  0.0              0.0              0.0  
trial_type.Finger                      1.0              0.0              0.0  
finger_vs_others                       0.0              0.0              0.0  
finger_vs_others.retest                0.0              0.0              0.0  
lips_vs_others                         0.0              0.0              0.0  
trial_type.Foot                        0.0              1.0              0.0

To do:

• Identify an appropriate data structure that will enable application of a matrix of this (or similar form)
• Determine what changes, if any, are needed to the outputs of get_contrasts()

Currently, the CLI and the fitlins library are very interlaced.
That is, it's currently not intuitive to use fitlins as a library that can be imported by another python program.

For example, default values for parameters come from argparse, and the function create_workflow takes a since opts argument, rather than explicitly stating the different inputs.

IMO, it would be useful to divorce these two a bit, so that create_workflow could be called without having to go through the CLI. This would make it easy for neuroscout-cli to wrap fitlinns without having to resort to using subprocess.

Thoughts?

(migrating this from pybids where I falsely posted it before) pinging @effigies (@yarikoptic promised me to give you a hydra account that I can finally share the data).

We've stripped our dataset to a bare minimum: 3 subjects, each with 2 runs, and a model file with only one event in X to reduce computation time. You can find the dataset on hydra in the directory
/data/movieloc/backup_store/publish-BIDSSacc on branch slim (please disregard the master branch for now - it is a couple of weeks behind and I don't want to push the currently rather messy state of it).
The model (found under models/movel_v3_smdl_autocon_splitconv_oneevent.json is evoked with

fitlins . Jan_28 'run' -m models/movel_v3_smdl_autocon_splitconv_oneevent.json --desc highpass --space 'MNI152NLin6Sym' -d $PWD -w 'Jan_28_wd' --n-cpus 3

{
    "name": "FEF_localizer",
    "Input": {
        "session": "movie"
        },
    "Steps": [
        {
            "Level": "run",
            "Model": {
                "X": [
                    "amplitude_.RIGHT"
                    ]
            },
            "Contrasts": [],
            "AutoContrasts": true,
            "Transformations": [{
                "Name": "Split",
                "Input": ["amplitude_"],
                "By": ["trial_type"]
                },
                {
                "Name": "Convolve",
                "Input": ["amplitude_.RIGHT"],
                "Model": "spm"
                  }]
            },
        {
            "Level": "subject",
            "AutoContrasts": true
        },
        {
            "Level": "dataset",
            "AutoContrasts": true
        }]
}

The model fails on level 3 (dataset) with

ValueError: A second level model requires a list with atleast two first level models or niimgs

after producing seemingly sensible output on the first (run) and second (subject) level. As far as I can judge, the files that the level 3 model should ingest have been created.

Traceback (most recent call last):
  File "/home/adina/Repos/nipype/nipype/pipeline/plugins/multiproc.py", line 69, in run_node
    result['result'] = node.run(updatehash=updatehash)
  File "/home/adina/Repos/nipype/nipype/pipeline/engine/nodes.py", line 473, in run
    result = self._run_interface(execute=True)
  File "/home/adina/Repos/nipype/nipype/pipeline/engine/nodes.py", line 1254, in _run_interface
    self.config['execution']['stop_on_first_crash'])))
  File "/home/adina/Repos/nipype/nipype/pipeline/engine/nodes.py", line 1176, in _collate_results
    (self.name, '\n'.join(msg)))
Exception: Subnodes of node: l3_model failed:
Subnode 0 failed
Error: Traceback (most recent call last):

  File "/home/adina/Repos/nipype/nipype/pipeline/engine/utils.py", line 99, in nodelist_runner
    result = node.run(updatehash=updatehash)

  File "/home/adina/Repos/nipype/nipype/pipeline/engine/nodes.py", line 473, in run
    result = self._run_interface(execute=True)

  File "/home/adina/Repos/nipype/nipype/pipeline/engine/nodes.py", line 557, in _run_interface
    return self._run_command(execute)

  File "/home/adina/Repos/nipype/nipype/pipeline/engine/nodes.py", line 637, in _run_command
    result = self._interface.run(cwd=outdir)

  File "/home/adina/Repos/nipype/nipype/interfaces/base/core.py", line 369, in run
    runtime = self._run_interface(runtime)

  File "/home/adina/Repos/fitlins/fitlins/interfaces/nistats.py", line 174, in _run_interface
    model.fit(input, design_matrix=design_matrix)

  File "/home/adina/env/fitlins/local/lib/python3.5/site-packages/nistats/second_level_model.py", line 164, in fit
    raise ValueError('A second level model requires a list with at'

ValueError: A second level model requires a list with atleast two first level models or niimgs

Reproducibility sits in a bar somewhere and laughs its ass of, but I'm trying to also give an overview of custom changes @yarikoptic and I made to the fitlins sourcecode. I don't see any particular relevance of the changes we made to the issue at hand (mostly hardcoding quickfixes for issues that erose), but then again, I'm certainly not the one to judge what is of relevance and what not ;-) , and an attempt to rerun the model would fail without the additional space choice and the selection of only one of two identical bold files being returned.

diff --git a/fitlins/cli/run.py b/fitlins/cli/run.py
index 28dcdba..f64e14e 100755
--- a/fitlins/cli/run.py
+++ b/fitlins/cli/run.py
@@ -85,15 +85,15 @@ def get_parser():
     g_bids.add_argument('--derivative-label', action='store', type=str,
                         help='execution label to append to derivative directory name')
     g_bids.add_argument('--space', action='store',
-                        choices=['MNI152NLin2009cAsym', ''],
+                        choices=['MNI152NLin2009cAsym', '', 'MNI152NLin6Sym'],
                         default='MNI152NLin2009cAsym',
                         help='registered space of input datasets. Empty value for no explicit space.')

diff --git a/fitlins/interfaces/bids.py b/fitlins/interfaces/bids.py
index 919742f..a7b98a9 100644
--- a/fitlins/interfaces/bids.py
+++ b/fitlins/interfaces/bids.py
@@ -177,7 +177,7 @@ class LoadBIDSModel(SimpleInterface):
         selectors = self.inputs.selectors
 
         analysis = Analysis(model=self.inputs.model, layout=layout)
-        analysis.setup(drop_na=False, desc='preproc', **selectors)
+        analysis.setup(drop_na=False, desc='highpass', space='MNI152NLin6Sym', **selectors)
         self._load_level1(runtime, analysis)
         self._load_higher_level(runtime, analysis)
 
@@ -198,25 +198,37 @@ class LoadBIDSModel(SimpleInterface):
[...]
-            if len(preproc_files) != 1:
-                raise ValueError('Too many BOLD files found')
+            # ATM we could get multiple entries for the same file
+            # see https://github.com/bids-standard/pybids/issues/350
+            if len(set(f.path for f in preproc_files)) != 1:
+                raise ValueError(
+                    'Too many (%d) BOLD files found: %s'
+                    % (len(preproc_files), ', '.join(preproc_files))
+                )
 
             fname = preproc_files[0].path
 

Do you have any idea what I am missing here to figure out why the dataset level does not work?

File "/src/fitlins/fitlins/workflows/base.py", line 57, in init_fitlins_wf
BIDSDataSink(base_directory=str(Path(base_dir) / 'reportlets' / 'fitlins'),
File "/opt/conda/envs/neuro/lib/python3.6/pathlib.py", line 979, in new
self = cls._from_parts(args, init=False)
File "/opt/conda/envs/neuro/lib/python3.6/pathlib.py", line 654, in _from_parts
drv, root, parts = self._parse_args(args)
File "/opt/conda/envs/neuro/lib/python3.6/pathlib.py", line 638, in _parse_args
a = os.fspath(a)
TypeError: expected str, bytes or os.PathLike object, not NoneType

The default value for work_dir in init_fitlins_wf is None (and in the CLI argparse). But this is fed into Path, which causes the error above.

When there is a session grouping, and it is not modeled, it seems that the group level images are output there.

For example, studyforrest has a session ses-movie, so all the group level images are under a folder ses-movie. When there is no session grouping, they would just be at the top level directory.

Shouldn't they be at the top level when the session is just ignored in the model? This makes it a bit inconsistent to know if they are group level images, and to locate them.

asking on behalf of @yarikoptic after we stumbled across https://github.com/nistats/nistats/issues/27 when we looked at how the higher level models are computed.

I'm getting a weird permissions problem. Fitlins is unable to create the output directory. I hadn't run into this issue before because in neuroscout-cli I extend your Dockerfile and switch the user to root. So the docker user is able to access my mounted volumes. But it seems with user neuro it's not able to.

docker run -v /tmp/out:/out -v /home/zorro/datasets/test_datalad:/data --rm -it poldracklab/fitlins /data /out dataset -p /data/life/fmriprep

If I chmod +666 /tmp/out fitlins is able to run.

I could see the flip side of this, that by running with the user set to root, it may create files in the host filesystem with the incorrect permissions. Maybe this is why neurodocker doesn't use root?

When outputting contrasts, building a path fails, because one of the entities is 'run' but it is not in the pattern.

This is probably due to bids-standard/pybids#156

Previously, 'run' was erroneously being left out of the entities that came back with the design_matrix, and thus you were not expecting it.