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

Potential pitfall: aparc/aseg outputs in standard space #1631

Closed
oesteban opened this issue May 10, 2019 · 5 comments · Fixed by #1636
Closed

Potential pitfall: aparc/aseg outputs in standard space #1631

oesteban opened this issue May 10, 2019 · 5 comments · Fixed by #1636
Assignees
Labels
bug effort: medium Estimated medium effort task impact: medium Estimated medium impact task
Milestone

Comments

@oesteban
Copy link
Member

The following section of fMRIPrep is arguably incorrect:

https://github.com/poldracklab/fmriprep/blob/a8ef9a3b1722dd667e6ca0fc0e06a1fd23219eb1/fmriprep/workflows/bold/resampling.py#L356-L388

The reason is that anatomical derivatives are being processed as though they were functional. I guess the rationale is that, when we write outputs on standard space with native grid, we retrieve the resolution information from the functional images.

IMHO, aparc/aseg might be useful to have in standard space, with native resolution if you will, but they should be written out to the anat/ folder, and the native resolution should be the T1w.

I believe these were generated for the CIFTI subworkflow. Since we will be revising that very soon, I'd propose to make this move now.

One additional detail to add is that these images are not correctly aligned at least with 1.4.0a1 (might be another one error from the refactor, or just wrong since the beginning).

WDTY @effigies ?

@oesteban oesteban added bug impact: medium Estimated medium impact task effort: medium Estimated medium effort task labels May 10, 2019
@oesteban oesteban added this to the 1.4.0 milestone May 10, 2019
@oesteban oesteban self-assigned this May 10, 2019
@oesteban
Copy link
Member Author

Any opinions on this, @effigies ?

@effigies
Copy link
Member

Ah, thanks for the ping...

They are anatomical derivatives, but they are in the BOLD space specifically to make it trivial to select voxels in the BOLD series. I don't see that there's much advantage to moving them to the T1w grid.

@oesteban
Copy link
Member Author

What if we just stop writing them?

@effigies
Copy link
Member

We got #1401 specifically because users needed them in MNI space. IDK. What's the motivation for removing them? Just conceptual clarity?

oesteban added a commit to oesteban/fmriprep that referenced this issue May 15, 2019
…tandard space

This PR addresses a problem dual to nipreps#1630 and closes nipreps#1631. The transforms being fed to the resamplers were not being `KeySelect`ed.
@oesteban
Copy link
Member Author

Okay, given #1401, I've gone through the debugging and fixing of the error.

However, I still don't see an acceptable use for these derivates other than testing whether transforms were correctly chained and applied or not. Using these files to extract time series in standard space should be a strongly discouraged use of these derivatives.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug effort: medium Estimated medium effort task impact: medium Estimated medium impact task
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants