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

ENH: Split SyN fieldmap estimates per-EPI #312

Merged
merged 2 commits into from
May 19, 2023

Conversation

effigies
Copy link
Member

@effigies effigies commented Dec 6, 2022

This is an experimental PR to undo the merging of individual EPI images which may not be well-aligned with respect to the phase-encoding direction. This will revert us to the basic collation method of fMRIPrep 20.2.x while preserving the fit-apply split introduced in fMRIPrep 21.0.0.

This experiment is worth performing before continuing to optimize b-spline approximation of the SyN-estimated field.

I've tried to write this with an eye to making it easier to plug in grouping strategies in the future.

First step in fully addressing #306.

@effigies effigies marked this pull request as draft December 6, 2022 21:40
@effigies
Copy link
Member Author

effigies commented Dec 7, 2022

When splitting, I get normal looking fields (only one shown, as they all look alike):

image

But the conversion to the individual subject space varies across images (I see two like each):

image
image

Unsurprisingly distortion correction looks much more reasonable when the run-space fieldmaps look reasonable (though neither is as bad as some failures we've seen):

Screenshot from 2022-12-07 08-17-39
Screenshot from 2022-12-07 08-18-21

What's odd is that all the input images have the same affine and the only difference I'm seeing in metadata is a few of the slice times.

@effigies
Copy link
Member Author

effigies commented Dec 7, 2022

Oh man. There's a collation problem here, where the low-frequency maps are associated with odd runs and high-frequency with even.

@effigies
Copy link
Member Author

effigies commented Dec 7, 2022

Addressed in #317.

@effigies
Copy link
Member Author

effigies commented Dec 8, 2022

This does not seem to be necessary for the test cases I'm looking at. I suggest we hold off until 23.0 to see whether problems are resolved for most users with our other updates. Then we could make this an option.

@effigies effigies marked this pull request as ready for review April 21, 2023 18:37
@effigies effigies modified the milestones: 2.3.0, 2.5.0 May 8, 2023
@effigies effigies merged commit b40d23a into nipreps:master May 19, 2023
@effigies effigies deleted the enh/decouple_epi branch May 19, 2023 01:50
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.

1 participant