Skip to content

Conversation

@oesteban
Copy link
Member

EstimateReferenceImage generates 4D references if 4D SBRefs are supplied as input. This patch checks the dimensionality of SBRefs and generates an average if the SBRef is 4D.

Fixes nipreps/fmriprep#1579.

Intimately related to #253, although I'm not sure the proposed resampling there is absolutely necessary.
It could be detrimental for bbregister in co-registration.
It is very likely necessary for head-motion correction with either mcflirt or 3dVolReg.

… 4D SBRefs are supplied as input.

This patch checks the dimensionality of SBRefs and generates an average if the SBRef is 4D.
Fixes nipreps/fmriprep#1579.
Intimately related to nipreps#253, although I'm not sure the proposed resampling there is absolutely necessary.
It could be detrimental for ``bbregister`` in co-registration.
It is very likely necessary for head-motion correction with either ``mcflirt`` or ``3dVolReg``.
@oesteban oesteban requested a review from effigies April 11, 2019 00:55
@oesteban
Copy link
Member Author

Also fixes nipreps/fmriprep#1463.

@effigies
Copy link
Member

Have we checked the spec to make sure sbrefs are legal to have as 4d?

Copy link
Member

@effigies effigies left a comment

Choose a reason for hiding this comment

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

Have we checked the spec to make sure sbrefs are legal to have as 4d?

I checked. It's never stated explicitly, so I guess this is legal...

This looks fine, overall. One minor code comment,. but TBH we're loading the whole data array anyway so it doesn't make much difference. Are you sure we don't just want the first volume, though, rather than taking the average?

@oesteban
Copy link
Member Author

Are you sure we don't just want the first volume, though, rather than taking the average?

The averaged should have much higher SNR without a cost in contrast. I think it makes more sense and computationally is not a big deal.

@oesteban oesteban merged commit b7d111c into nipreps:master Apr 11, 2019
@oesteban oesteban deleted the fix/fmriprep-issue-1579 branch April 11, 2019 06:46
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.

2 participants