ENH: generate CIFTI derivatives #1001

mgxd · 2018-02-21T17:56:52Z

converts bold/gii data to CIFTI dtseries file
- fsaverage5/fsaverage6 surfaces + MNI2009NLin2009cAsym subcortical volumes
adds compress input to DerivativesDataSink
increases required niworkflows to 0.3.7

TODOs

complete interface
add to bold workflow
add testing

To be added in another PR

create HCP 91282 greyordinate space CIFTIs (Support HCP-compatible CIFTI outputs #1025)
support fsnative surface + subject space aparc+aseg
- number of indices in fsaverage5 (~10k) vs fsnative (+100k) will make these files fairly large

effigies

This looks pretty reasonable from my limited knowledge of CIFTI. Could you add the --cifti-output flag to the ds005 tests?

https://github.com/poldracklab/fmriprep/blob/d13d39ef97a613c1901c0cc916834aca681f540e/.circleci/config.yml#L283-L289

effigies · 2018-02-22T15:34:41Z

fmriprep/interfaces/cifti.py

+    subjects_dir = Directory(mandatory=True, desc="FreeSurfer SUBJECTS_DIR")
+    TR = traits.Float(mandatory=True, desc="repetition time")
+    gifti_files = traits.List(File(exists=True), mandatory=True,
+                              desc="surface geometry files")


Probably worth specifying that this should be length 2 with order (left, right).

effigies · 2018-02-22T15:38:08Z

fmriprep/interfaces/cifti.py

+                                                    'label',
+                                                    '*h.aparc.annot')))
+        # TODO: fetch label_file
+        return annotation_files


Does it make sense to use a FreeSurferSource to find the aparc.annot files?

Also, do we want to consider the possibility people will want to use DKTatlas or a2009s (at least to the extent of being aware of whether we're making a move in that direction easy or hard)?

We could but we'd still have to parse the results - I can change if you feel strongly.

Could you elaborate on your second point? Use how?

FreeSurfer now automatically produces

% ls /data/out/ds000114/derivatives/freesurfer/sub-07/label/lh.aparc*.annot /data/out/ds000114/derivatives/freesurfer/sub-07/label/lh.aparc.a2009s.annot /data/out/ds000114/derivatives/freesurfer/sub-07/label/lh.aparc.annot /data/out/ds000114/derivatives/freesurfer/sub-07/label/lh.aparc.DKTatlas.annot

It seems plausible that someone might want to generate their CIFTI files with labels from another parcellation. Again, not necessarily that we need to code it up right now, but just to consider how easy it would be to augment.

(It may also be that I'm horribly wrong and CIFTI pretty much demands vanilla aparc.)

effigies · 2018-02-22T15:38:43Z

fmriprep/interfaces/cifti.py

+            self.gifti_files,
+            self.volume_target,
+            self.surface_target,
+            self.TR)


self.inputs.*

effigies · 2018-02-22T15:38:54Z

fmriprep/interfaces/cifti.py

+
+    def _fetch_data(self):
+        """Converts inputspec to files"""
+        if self.surface_target == "fsnative":


self.inputs.surface_target

effigies · 2018-02-22T15:39:03Z

fmriprep/interfaces/cifti.py

+            raise NotImplementedError
+
+        annotation_files = sorted(glob(os.path.join(self.subjects_dir,
+                                                    self.surface_target,


self.inputs.*

effigies · 2018-02-22T15:47:27Z

fmriprep/workflows/bold/base.py

+                    ('targets.out', 'surface_target'),
+                    ('outputnode.surfaces', 'gifti_files')]),
+                (inputnode, cifti, [('subjects_dir', 'subjects_dir')]),
+                (bold_mni_trans_wf, outputnode, [('outputnode.bold_mni', 'bold_mni')]),


I suspect this is meant to be:

(cifti, outputnode, [('out_file', 'bold_cifti')])

effigies · 2018-02-22T15:49:01Z

fmriprep/interfaces/cifti.py

+            model_type = "CIFTI_MODEL_TYPE_SURFACE"
+            # use the corresponding annotation
+            hemi = structure.split('_')[-1][0]
+            annots = [fl for fl in annotation_files if ".aparc.annot" in fl]


While probably fine, this could be made a little more precise:

annots = [fl for fl in annotation_files if fl.endswith('.aparc.annot')]

effigies · 2018-02-22T15:49:53Z

fmriprep/interfaces/cifti.py

+                     else nb.freesurfer.read_annot(annots[1]))
+            # currently only supports L/R cortex
+            gii = (nb.load(gii_files[0]) if hemi == "LEFT"
+                   else nb.load(gii_files[1]))


gii = nb.load(gii_files[hemi == 'RIGHT'])

effigies · 2018-02-22T15:53:53Z

fmriprep/interfaces/cifti.py

+    output_spec = GenerateCiftiOutputSpec
+
+    def _run_interface(self, runtime):
+        self.annotation_files, self.label_file = self._fetch_data()


No clear reason to assign these to instance variables.

effigies · 2018-02-22T15:54:06Z

fmriprep/interfaces/cifti.py

+            hemi = structure.split('_')[-1][0]
+            annots = [fl for fl in annotation_files if ".aparc.annot" in fl]
+            annot = (nb.freesurfer.read_annot(annots[0]) if hemi == "LEFT"
+                     else nb.freesurfer.read_annot(annots[1]))


annot = nb.freesurfer.read_annot(annots[hemi == 'RIGHT'])

oesteban · 2018-02-22T18:55:03Z

we should probably store the label_file template to make this process faster - perhaps to the OSF project used in niworkflows?

👍 I'll give you access to the OSF repository. What is your username/orcid/handle?

mgxd · 2018-02-22T20:05:56Z

@oesteban orcid # 0000-0002-7252-7771

satra · 2018-02-22T21:28:22Z

fmriprep/interfaces/cifti.py

+    'CIFTI_STRUCTURE_CAUDATE_LEFT': [11],
+    'CIFTI_STRUCTURE_CAUDATE_RIGHT': [50],
+    'CIFTI_STRUCTURE_CEREBELLAR_WHITE_MATTER_LEFT': [7],
+    'CIFTI_STRUCTURE_CEREBELLAR_WHITE_MATTER_RIGHT': [46],


i don't think we need the white matter voxels included.

satra · 2018-02-22T21:30:19Z

fmriprep/interfaces/cifti.py

+    'CIFTI_STRUCTURE_CEREBELLUM_LEFT': [6],
+    'CIFTI_STRUCTURE_CEREBELLUM_RIGHT': [45],
+    'CIFTI_STRUCTURE_DIENCEPHALON_VENTRAL_LEFT': [28],
+    'CIFTI_STRUCTURE_DIENCEPHALON_VENTRAL_RIGHT': [60],


or the diencephalon at present. i'm not sure these are labeled by the atlas.

I used the labels provided on mindboggle's page - I'll remove these though

ok you can leave these in place. do you have an example file on openmind that i can take a look at?

satra · 2018-02-22T21:32:01Z

fmriprep/interfaces/cifti.py

+    subjects_dir = Directory(mandatory=True, desc="FreeSurfer SUBJECTS_DIR")
+    TR = traits.Float(mandatory=True, desc="repetition time")
+    gifti_files = traits.List(File(exists=True), mandatory=True,
+                              desc="list of surface geometry files (length 2 with order [L,R])")


@effigies - does the gifti file contain any info about the surface target, in which case surface_target should be removed and just derived from the gifti file.

So these are functional time series, and it does not appear so.

% mris_info /data/out/ds000114/derivatives/fmriprep/sub-07/ses-test/func/sub-07_ses-test_task-fingerfootlips_bold_space-fsaverage5.L.func.gii gifti_image struct version = 1.0 numDA = 184 gim->meta nvpairs struct, len = 4 : nvpair: 'AnatomicalStructurePrimary' = 'CortexLeft' nvpair: 'UserName' = 'root' nvpair: 'Date' = 'Mon Oct 16 18:34:03 2017' nvpair: 'gifticlib-version' = 'gifti library version 1.09, 28 June, 2010' gim->labeltable giiLabelTable struct, len = 0 : -------------------------------------------------- gim->darray[0] giiDataArray struct intent 2001 = NIFTI_INTENT_TIME_SERIES datatype 16 = NIFTI_TYPE_FLOAT32 ind_ord 1 = RowMajorOrder num_dim = 1 dims = 10242, 0, 0, 0, 0, 0 encoding 3 = GZipBase64Binary endian 2 = LittleEndian ext_fname = ext_offset = 0 darray->meta nvpairs struct, len = 1 : nvpair: 'TimeStep' = '2500.000000' darray->coordsys giiCoordSystem struct dataspace = NIFTI_XFORM_UNKNOWN xformspace = NIFTI_XFORM_UNKNOWN xform[0] : 1.000000 0.000000 0.000000 0.000000 xform[1] : 0.000000 1.000000 0.000000 0.000000 xform[2] : 0.000000 0.000000 1.000000 0.000000 xform[3] : 0.000000 0.000000 0.000000 1.000000 data = <set> nvals = 10242 nbyper = 4 numCS = 1 darray->ex_atrs nvpairs struct, len = 0 : -------------------------------------------------- -------------------------------------------------- gim->darray[1] giiDataArray struct intent 2001 = NIFTI_INTENT_TIME_SERIES datatype 16 = NIFTI_TYPE_FLOAT32 ind_ord 1 = RowMajorOrder num_dim = 1 dims = 10242, 0, 0, 0, 0, 0 encoding 3 = GZipBase64Binary endian 2 = LittleEndian ext_fname = ext_offset = 0 darray->meta nvpairs struct, len = 1 : nvpair: 'TimeStep' = '2500.000000' darray->coordsys giiCoordSystem struct dataspace = NIFTI_XFORM_UNKNOWN xformspace = NIFTI_XFORM_UNKNOWN xform[0] : 1.000000 0.000000 0.000000 0.000000 xform[1] : 0.000000 1.000000 0.000000 0.000000 xform[2] : 0.000000 0.000000 1.000000 0.000000 xform[3] : 0.000000 0.000000 0.000000 1.000000 data = <set> nvals = 10242 nbyper = 4 numCS = 1 darray->ex_atrs nvpairs struct, len = 0 : -------------------------------------------------- [...] -------------------------------------------------- gim->darray[183] giiDataArray struct intent 2001 = NIFTI_INTENT_TIME_SERIES datatype 16 = NIFTI_TYPE_FLOAT32 ind_ord 1 = RowMajorOrder num_dim = 1 dims = 10242, 0, 0, 0, 0, 0 encoding 3 = GZipBase64Binary endian 2 = LittleEndian ext_fname = ext_offset = 0 darray->meta nvpairs struct, len = 1 : nvpair: 'TimeStep' = '2500.000000' darray->coordsys giiCoordSystem struct dataspace = NIFTI_XFORM_UNKNOWN xformspace = NIFTI_XFORM_UNKNOWN xform[0] : 1.000000 0.000000 0.000000 0.000000 xform[1] : 0.000000 1.000000 0.000000 0.000000 xform[2] : 0.000000 0.000000 1.000000 0.000000 xform[3] : 0.000000 0.000000 0.000000 1.000000 data = <set> nvals = 10242 nbyper = 4 numCS = 1 darray->ex_atrs nvpairs struct, len = 0 : -------------------------------------------------- gifti_image struct swapped = 0 compressed = 1 -- darray totals: 7 MB gim->ex_atrs nvpairs struct, len = 0 : ==================================================

satra · 2018-02-22T21:33:11Z

fmriprep/interfaces/cifti.py

+    bold_file = File(mandatory=True, exists=True, desc="input BOLD file")
+    volume_target = traits.Enum("MNI152NLin2009cAsym", mandatory=True, usedefault=True,
+                                desc="CIFTI volumetric output space")
+    surface_target = traits.Enum("fsaverage5", "fsaverage6", "fsaverage", mandatory=True,


probably remove fsaverage for now as well since it's going to be really large.

This seems fine to me to leave, since fsaverage is a valid (though off-by-default) target for FMRIPREP.

i'm just worried about the file size - also the reason i suggested leaving out cerebellar wm voxels.

remove unused CIFTI structures, add link for OSF label storage

mgxd · 2018-02-27T20:55:52Z

sits on top of nipreps/niworkflows#228

mgxd · 2018-02-28T15:45:40Z

okay - this might be ready to go, but will require releasing and using current niworkflows to allow users to pull the label atlas

…/cifti [skip ci]

effigies · 2018-02-28T15:59:04Z

Oh, sorry. You'll need to pin the niworkflows commit. I can do that on your branch, as it's probably the easiest way to show you how.

Edit: Looks like you got the style fixes.

satra · 2018-02-28T16:00:22Z

@oesteban - while this is fine for now, i did ask @mgxd to put interfaces into nipype itself, so this may move over into misc.

oesteban · 2018-02-28T18:32:53Z

I'll cut a new niworkflows release. When migration to nipype is done we'll release niworkflows again.

oesteban · 2018-02-28T20:56:34Z

@mgxd niworkflows-0.3.5 has been released just now.

effigies · 2018-04-06T15:11:09Z

Sorry about not formatting as a single review. I'm done now.

mgxd · 2018-04-10T18:57:01Z

@effigies re: #1001 (comment), it looks like inputnode is never added to the workflow within init_sdc_wf if only epi fieldmap types are available, so the line https://github.com/poldracklab/fmriprep/blob/1755986606806207fdc665a2a469aff2cbfd2dba/fmriprep/workflows/bold/base.py#L387 should fail. I'll add a patch within this PR

effigies · 2018-04-10T19:01:04Z

Oh, wow. Good catch.

oesteban · 2018-04-10T20:46:33Z

the inputnode should be added in here:

https://github.com/poldracklab/fmriprep/blob/1755986606806207fdc665a2a469aff2cbfd2dba/fmriprep/workflows/fieldmap/base.py#L158

for the case no fieldmap is found. Maybe it is only a case of real fieldmaps?

oesteban · 2018-04-10T20:47:35Z

Sorry, I misunderstood. You are right!

@mgxd

As a result of refactoring, this node got disconnected for that particular use-case. This PR fixes that (nipreps#1001 (comment)) Thanks @mgxd for spotting this problem.

mgxd · 2018-04-17T17:15:08Z

i think we're good to go 👍 !

effigies

Okay, one last iteration. Feel free to argue it out, but we feel fairly strongly that the resampling issue should be resolved before merging.

@satra Do you want to have a look at the BOLD->label resampling? I think you might have a little more CIFTI context and can make sure we haven't fooled ourselves into approaching this wrongly.

effigies · 2018-04-20T14:24:53Z

fmriprep/interfaces/cifti.py

+        label_img = nb.load(label_file)
+        if not bold_img.shape[:3] == label_img.shape:
+            # we need these images to have the same resolution
+            bold_img = resample_to_img(bold_img, label_img, interpolation="nearest")


Okay, we had a discussion over this line, and we think it should be fine if you just drop the interpolation='nearest'. The issue is that in many cases this is going to be upsampling BOLD data to 2mm^3 voxels, which should really be interpolated.

effigies · 2018-04-20T14:26:09Z

fmriprep/interfaces/cifti.py

+
+        bold_img = nb.load(bold_file)
+        label_img = nb.load(label_file)
+        if not bold_img.shape[:3] == label_img.shape:


You can drop this check. nilearn.image.resample_to_img will just return a copy if the shape and affine match.

So I'd actually write the whole loading section in two lines:

label_img = nb.load(label_file) bold_img = resample_to_img(bold_file, label_img)

effigies · 2018-04-20T19:54:11Z

Awesome. In it goes.

chrisgorgo · 2018-04-21T03:12:16Z

This is great! Thank you so much @mgxd and everyone else for the feedback.

mgxd added 2 commits February 21, 2018 11:59

enh: generate CIFTI derivatives

b12072d

[ci skip] enh: start adding to workflow

e1b57d5

effigies reviewed Feb 22, 2018

View reviewed changes

fix: some missing connections, addressing @effigies review

0230ab4

satra reviewed Feb 22, 2018

View reviewed changes

mgxd added 3 commits February 23, 2018 11:16

test: add cifti to ds005 testing

d222aef

enh+fix: save output as valid CIFTI dtseries

96cff8c

remove unused CIFTI structures, add link for OSF label storage

sty: removed blank line [skip ci]

270d16e

Merge branch 'master' into enh/cifti

622ce39

mgxd added 2 commits February 28, 2018 10:58

sty: flake8 fixes [skip ci]

4dd53c8

Merge branch 'enh/cifti' of https://github.com/mgxd/fmriprep into enh…

00e8e74

…/cifti [skip ci]

effigies and others added 3 commits February 28, 2018 11:04

PIN: poldracklab/niworkflows master

87f29a4

Merge remote-tracking branch 'upstream/master' into enh/cifti

3606d6e

fix: variable name

f1036be

mgxd added 4 commits February 28, 2018 14:14

fix: add cifti_output to function docstring

c214e28

enh: upgrade to niworkflows 0.3.5

1dff620

fix: connection syntax

da37487

PIN: niworkflows 0.3.5 until pip installable

f2949bf

enh: fix resource name, add docstring to create_cifti_image

a8a9bb7

mgxd added 3 commits April 6, 2018 11:37

sty: make surface option exclusivity clearer

6b9d1e9

sty: keep node name consistent with variable

4f179b6

fix: remove duplicate arguments

61b2de6

oesteban mentioned this pull request Apr 10, 2018

[FIX] Connect inputnode to SDC for pepolar images #1046

Merged

mgxd added 7 commits April 11, 2018 11:35

merge upstream

6b9bc58

enh: output supplementary json for cifti variants

cd87258

sty: forgotten comma

1e67ce6

fix: make suffix iterable

6570783

sty: flake8 indentation

06bdd61

fix: missing connection

277cbc4

tst: ds005 outputs

f4d83e3

oesteban mentioned this pull request Apr 12, 2018

Revise sampling of subcortical volume #1053

Closed

merge with upstream master

e51fa5e

mgxd added 2 commits April 19, 2018 14:25

ci: add cifti option to rerun

6f64800

ci: add cifti derivatives to partial outputs

be41721

effigies reviewed Apr 20, 2018

View reviewed changes

fix: use continuous interpolation when resampling

b1ff492

effigies merged commit 677c556 into nipreps:master Apr 20, 2018

1.3.0 automation moved this from In progress to Done Apr 20, 2018

This was referenced Apr 27, 2018

[URGENT] Degenerate derivatives #1075

Closed

Generate CIFTI derivatives #794

Closed

ENH: generate CIFTI derivatives #1001

ENH: generate CIFTI derivatives #1001

Conversation

mgxd commented Feb 21, 2018 • edited Loading

TODOs

To be added in another PR

effigies left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

oesteban commented Feb 22, 2018

mgxd commented Feb 22, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mgxd commented Feb 27, 2018

mgxd commented Feb 28, 2018

effigies commented Feb 28, 2018 • edited Loading

satra commented Feb 28, 2018

oesteban commented Feb 28, 2018

oesteban commented Feb 28, 2018

effigies commented Apr 6, 2018

mgxd commented Apr 10, 2018

effigies commented Apr 10, 2018

oesteban commented Apr 10, 2018

oesteban commented Apr 10, 2018

mgxd commented Apr 17, 2018

effigies left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

effigies commented Apr 20, 2018

chrisgorgo commented Apr 21, 2018

mgxd commented Feb 21, 2018 •

edited

Loading

effigies commented Feb 28, 2018 •

edited

Loading