Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fMRIVolume Related Pull Request — Slice timing correction #139

Closed

Conversation

sivcek
Copy link
Contributor

@sivcek sivcek commented Oct 3, 2019

Addition of slice timing correction functionality.

@sivcek sivcek changed the title Qunex fmrivolume stc fMRIVolume Related Pull Request — Slice timing correction Oct 4, 2019
…the filename of the reoriented image to *_orig_reorient.
Copy link
Contributor

@glasserm glasserm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please put a warning when using slice timing correction to this effect (should also appear next to the options in the Batch script):

WARNING: This legacy option of slice timing correction is performed before motion correction (as is typically done in legacy-style brain imaging) and thus assumes that the brain is motionless. Errors in temporal interpolation will occur in the presence of head motion and may also disrupt data quality measures as shown in Power et al 2017 PLOS One "Temporal interpolation alters motion in fMRI
scans: Magnitudes and consequences for artifact detection." Slice timing correction and motion correction would ideally be performed simultaneously; however, this is not currently supported by any major software tool. HCP-Style fast TR fMRI data acquisitions (TR<=1s) avoid the need for slice timing correction, provide major advantages for fMRI denoising, and are recommended.

@sivcek
Copy link
Contributor Author

sivcek commented Oct 7, 2019

Added the warning.

Comment on lines 348 to 349
# use FSL's fslreorient2std for reorienting the image to match the approximate orientation of the standard template images MNI152
# this makes the single-band processing more robust to registration problems
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This won't work. You can't reorient the slice acquisition axis without also accounting for that as part of STC. I see that slicetimer has an option (-d, --direction) to set the direction of slice acquisition, but if you want to use that, you will need to add code that ascertains what is the axes of the slice acquisition after fslreorient2std has been applied. (If you have previously processed any non-axially acquired data using this approach, then the STC applied to that data is completely wrong).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it ok if we do reorient after slice timing correction?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As regards STC itself, yes, but not in the broader context of fMRIVolume overall, where:

  1. changing the orientation changes the UnwarpDir.
  2. I'm not immediately sure of the implications on the SEFMs either.

This is a complicated issue if you want the reorientation to be part of the pipeline code itself.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm. What if we currently limit it to only those data that do not have any DC method available?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As this is quite common intersection in legacy datasets with SB bolds with no DC, so it would cover most cases where fslreorient might help.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Chad and Takuya said they hand-reoriented (sometimes needing to relabel the axes) the NHP inputs before using them, so the NHP pipelines never need to deal with non-axial data.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is probably okay, though we should think about how this would interact with fieldmap-less distortion correction if it is implemented.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can I assume the consensus is we limit it to the data w/o available DC correction method?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For now, I think it is either that, or you eliminate the fslreorient2std part entirely. Regardless, people need to be aware of the reorienting, so I'd include a prominent comment at that section of the code, and also issue a log_Warn on the issue.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did that. @mharms can you please check the note and warning and suggest changes if necessary.

Copy link
Contributor

@mharms mharms left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can't reorient the data prior to STC without also accounting for that.

@glasserm glasserm requested a review from mharms October 12, 2019 00:59
@sivcek
Copy link
Contributor Author

sivcek commented Nov 7, 2019

Joined with other fMRIVolume related PRs into PR #156

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants