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

FIX: ensure fieldwarp == identity during resampling of processed multi-echo data #2730

Merged
merged 2 commits into from
Mar 4, 2022

Conversation

markushs
Copy link
Contributor

@markushs markushs commented Mar 3, 2022

Changes proposed in this pull request

Likely fixes issues reported in #2727

Documentation that should be reviewed

No

avoid including fieldwarp transformations in resampling of unwarped multi-echo data
@effigies
Copy link
Member

effigies commented Mar 4, 2022

@oesteban Could I get a second pair of eyes on this? I think it's right, but I want to be extra careful.

@markushs Are you able to test this patch on your installation? Do you need any guidance?

@markushs
Copy link
Contributor Author

markushs commented Mar 4, 2022

@markushs Are you able to test this patch on your installation? Do you need any guidance?

@effigies I'll try to bind an edited version of base.py into my singularity image. Should work, but as this image reflects "vanilla" 21.0.1 I won't be able to test the patch against newly merged stuff.

@markushs
Copy link
Contributor Author

markushs commented Mar 4, 2022

My tests appear successful.
*_space-T1w*_bold.nii.gz-files are now identical to volumes in native functional "scanner"-space transformed to space-T1w using the fmriprep-generated *_from-scanner_to-T1w_mode-image_xfm.txt.

command.txt in bold_t1_trans_wf/bold_to_t1w_transform looks correct for all volumes, with 2x --transform identity (i added the backslashes for readibility):
image

Not sure what the empty _dpop function in the now deleted lines was doing, but everything looks good without, so maybe consider also getting rid of the function definition?

@effigies
Copy link
Member

effigies commented Mar 4, 2022

Thanks for confirming!

Copy link
Member

@oesteban oesteban left a comment

Choose a reason for hiding this comment

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

LGTM (opened #2732 as a reminder for myself)

@effigies effigies added this to the 21.0.2 milestone Mar 4, 2022
@effigies effigies merged commit 944bfa9 into nipreps:maint/21.0.x Mar 4, 2022
@markushs markushs deleted the me_fieldwarpfix branch March 29, 2022 12:17
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.

3 participants