ENH: Save scanner-to-T1w xform to outputs

[tsalo]

Closes #1184.

Open questions:

• Should tfm transforms be supported in DerivativesDatasink?
  • Answer: Replace tfm with txt.
• Do we want the native-to-T1w transform even when T1w isn't one of the desired spaces?
  • Answer: Yes.

Changes proposed in this pull request

• Incorporate bold2anat_xfm and anat2bold_xfm into functional derivatives workflow. This will save these transforms only when T1w is a desired output space.

Documentation that should be reviewed

I added the transforms to the "Outputs of fMRIPrep" page.

[tsalo]

The current failure in the ds005 test seems to stem from the fact that the transform being written out has a tfm extension instead of txt or hd5 (thanks to @akimbler for noticing this). Here's the relevant log:

[Node] Running "ds_ref_t1w_warp" ("fmriprep.interfaces.DerivativesDataSink")
200804-21:56:04,678 nipype.workflow WARNING:
	 Storing result file without outputs
200804-21:56:04,679 nipype.workflow WARNING:
	 [Node] Error on "fmriprep_wf.single_subject_01_wf.func_preproc_task_mixedgamblestask_run_02_wf.func_derivatives_wf.ds_ref_t1w_warp" (/scratch/fmriprep_wf/single_subject_01_wf/func_preproc_task_mixedgamblestask_run_02_wf/func_derivatives_wf/ds_ref_t1w_warp)
200804-21:56:04,680 nipype.workflow ERROR:
	 Node ds_ref_t1w_warp failed to run on host 2eec16105d58.
200804-21:56:04,690 nipype.workflow ERROR:
	 Saving crash info to /out/fmriprep/sub-01/log/20200804-210721_3c8fede0-ff9a-4e24-8a11-f1d93226c22c/crash-20200804-215604-UID1001-ds_ref_t1w_warp-428bc157-af06-45f2-82f3-a67937241be4.txt
Traceback (most recent call last):
  File "/usr/local/miniconda/lib/python3.7/site-packages/nipype/pipeline/plugins/legacymultiproc.py", line 432, in _send_procs_to_workers
    self.procs[jobid].run(updatehash=updatehash)
  File "/usr/local/miniconda/lib/python3.7/site-packages/nipype/pipeline/engine/nodes.py", line 516, in run
    result = self._run_interface(execute=True)
  File "/usr/local/miniconda/lib/python3.7/site-packages/nipype/pipeline/engine/nodes.py", line 635, in _run_interface
    return self._run_command(execute)
  File "/usr/local/miniconda/lib/python3.7/site-packages/nipype/pipeline/engine/nodes.py", line 741, in _run_command
    result = self._interface.run(cwd=outdir)
  File "/usr/local/miniconda/lib/python3.7/site-packages/nipype/interfaces/base/core.py", line 397, in run
    runtime = self._run_interface(runtime)
  File "/usr/local/miniconda/lib/python3.7/site-packages/niworkflows/interfaces/bids.py", line 535, in _run_interface
    raise ValueError(f"Could not build path with entities {out_entities}.")
ValueError: Could not build path with entities {'subject': '01', 'task': 'mixedgamblestask', 'run': 2, 'suffix': 'xfm', 'datatype': 'func', 'extension': 'tfm', 'from': 'native', 'to': 'T1w', 'mode': 'image'}.

[akimbler]

The current failure in the ds005 test seems to stem from the fact that the transform being written out has a tfm extension instead of txt or hd5 (thanks to @akimbler for noticing this). Here's the relevant log:

[Node] Running "ds_ref_t1w_warp" ("fmriprep.interfaces.DerivativesDataSink")
200804-21:56:04,678 nipype.workflow WARNING:
	 Storing result file without outputs
200804-21:56:04,679 nipype.workflow WARNING:
	 [Node] Error on "fmriprep_wf.single_subject_01_wf.func_preproc_task_mixedgamblestask_run_02_wf.func_derivatives_wf.ds_ref_t1w_warp" (/scratch/fmriprep_wf/single_subject_01_wf/func_preproc_task_mixedgamblestask_run_02_wf/func_derivatives_wf/ds_ref_t1w_warp)
200804-21:56:04,680 nipype.workflow ERROR:
	 Node ds_ref_t1w_warp failed to run on host 2eec16105d58.
200804-21:56:04,690 nipype.workflow ERROR:
	 Saving crash info to /out/fmriprep/sub-01/log/20200804-210721_3c8fede0-ff9a-4e24-8a11-f1d93226c22c/crash-20200804-215604-UID1001-ds_ref_t1w_warp-428bc157-af06-45f2-82f3-a67937241be4.txt
Traceback (most recent call last):
  File "/usr/local/miniconda/lib/python3.7/site-packages/nipype/pipeline/plugins/legacymultiproc.py", line 432, in _send_procs_to_workers
    self.procs[jobid].run(updatehash=updatehash)
  File "/usr/local/miniconda/lib/python3.7/site-packages/nipype/pipeline/engine/nodes.py", line 516, in run
    result = self._run_interface(execute=True)
  File "/usr/local/miniconda/lib/python3.7/site-packages/nipype/pipeline/engine/nodes.py", line 635, in _run_interface
    return self._run_command(execute)
  File "/usr/local/miniconda/lib/python3.7/site-packages/nipype/pipeline/engine/nodes.py", line 741, in _run_command
    result = self._interface.run(cwd=outdir)
  File "/usr/local/miniconda/lib/python3.7/site-packages/nipype/interfaces/base/core.py", line 397, in run
    runtime = self._run_interface(runtime)
  File "/usr/local/miniconda/lib/python3.7/site-packages/niworkflows/interfaces/bids.py", line 535, in _run_interface
    raise ValueError(f"Could not build path with entities {out_entities}.")
ValueError: Could not build path with entities {'subject': '01', 'task': 'mixedgamblestask', 'run': 2, 'suffix': 'xfm', 'datatype': 'func', 'extension': 'tfm', 'from': 'native', 'to': 'T1w', 'mode': 'image'}.

I think this is caused on the niworkflows side of things, specifically this.

[tsalo]

I'll see if limiting to runs with T1w-space outputs resolves the issue by not dealing with tfm xforms at all. If it doesn't, then I'll have to query the devs about tfm xforms and whether they should be supported or not.

[effigies]

• Should tfm transforms be supported in DerivativesDatasink?

What does tfm mean in this context?

• Do we want the native-to-T1w transform even when T1w isn't one of the desired spaces?

Yes, I think it's a good idea. Transforms are cheap. I would err on the side of saving too many transforms and too few resampled BOLD series.

---

I don't think native has any meaning in BIDS. What about scanner?

https://bids-specification.readthedocs.io/en/stable/99-appendices/08-coordinate-systems.html

---

Otherwise looks good.

[tsalo]

What does tfm mean in this context?

I don't know why tfm is popping up, but an error that was occurring with ds005 involved tfm as the extension. That extension isn't supported within the niworkflows nipreps.json.

Yes, I think it's a good idea. Transforms are cheap. I would err on the side of saving too many transforms and too few resampled BOLD series.

Okay, this will involve a corresponding PR to niworkflows, so it'll be a couple of days, I think.

I don't think native has any meaning in BIDS. What about scanner?

The transform relates to the BOLD reference image, so it's not truly "native" or scanner space. Would reference work?

[oesteban]

I don't know why tfm is popping up, but an error that was occurring with ds005 involved tfm as the extension. That extension isn't supported within the niworkflows nipreps.json

What is the error, exactly? - the .tfm extension should be replaceable by .txt.

The transform relates to the BOLD reference image, so it's not truly "native" or scanner space. Would reference work?

For NIfTI scanner means whatever coordinates the scanner registered. So assuming that the BOLD reference has the same affine information as the BOLD run it refers, then it is correct to say it has scanner space.

[tsalo]

What is the error, exactly? - the .tfm extension should be replaceable by .txt.

You can see the error in this comment: #2233 (comment)
If tfm can be replaced with txt, I can handle that in the workflow I think.

For NIfTI scanner means whatever coordinates the scanner registered. So assuming that the BOLD reference has the same affine information as the BOLD run it refers, then it is correct to say it has scanner space.

Okay, I can use scanner instead then.

[oesteban]

If tfm can be replaced with txt

Yes, that should work out

[oesteban]

One possibility here, which I think is very unclear from the BIDS-Derivs draft of transforms, is whether the "default" value can be omitted (similarly to why we omit space- when writing the bold file):

fmriprep/sub-100185/func/sub-100185_task-machinegame_run-1_to-T1w_mode-image_xfm.txt

WDYT @effigies ?

[oesteban]

that also covers the inverse:

fmriprep/sub-100185/func/sub-100185_task-machinegame_run-1_from-T1w_mode-image_xfm.txt

[effigies]

I think I'd rather be explicit. I see the sense in not adding an unnecessary space-scanner to a file that hasn't been moved into another space, but a transformation no longer has its own implicit space.

[effigies]

Now that I'm looking at that page again, should the T1w space be individual? That would obviously be a bigger change than this PR.

[oesteban]

Doesn't it say that individual means anatomical reference? ( https://bids-specification.readthedocs.io/en/stable/99-appendices/08-coordinate-systems.html#nonstandard-coordinate-system-identifiers )

…

On Fri, Aug 7, 2020, 23:18 Chris Markiewicz ***@***.***> wrote: Now that I'm looking at that page again, should the T1w space be individual? — You are receiving this because your review was requested. Reply to this email directly, view it on GitHub <#2233 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAESDRQFFFW2VTG4G6VK54LR7RVSVANCNFSM4PURR6RQ> .

[effigies]

Which is the T1w, right? At least when fmriprep constructs the reference.

[oesteban]

As I interpret it, from-individual_to-T1w can only designate the identity transform

…

On Sat, Aug 8, 2020, 00:10 Chris Markiewicz ***@***.***> wrote: Which is the T1w, right? At least when fmriprep constructs the reference. — You are receiving this because your review was requested. Reply to this email directly, view it on GitHub <#2233 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAESDRVO6X4STYO4DQYAJADR7R3T5ANCNFSM4PURR6RQ> .

[tsalo]

@oesteban, I think @effigies was proposing to change T1w to individual, so it would be from-scanner_to-individual.

[effigies]

Yes. Thanks for clarifying, @tsalo. To be clear, the proposal is to change the T1w space to individual throughout the derivatives. It is beyond this PR, but I thought I should bring it up, while we're taking about it.

[tsalo]

I opened #2234 for the space terminology question, since it's not something I'd be able to do within this PR.

[oesteban]

Thanks much for this.