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

syn preprocessing epi_reference_wf does not distinguish between different matrix sizes #352

Closed
eilidhmacnicol opened this issue Apr 20, 2023 · 6 comments
Labels
bug Something isn't working

Comments

@eilidhmacnicol
Copy link
Contributor

What happened?

I have 1 rs fMRI and 3 task fMRI for a given subject. The rs fMRI has a longer TR (1.5 secs) and more slices (36) than the task (1 sec; 24 slices). Despite whether a task filter is passed to fMRIPrep, find_estimators includes all available bold images as the source images because the task filter is not passed to find_estimators (which is admittedly a separate problem, so I can open a relevant issue in fmriprep if this is the case).

However, even if all the bold images were passed, I'd expect init_syn_preprocessing_wf to either a) check that all images have the same shape or b) create an EPI reference for each image shape rather than bundling them all together.

The resulting shape is the same as the rs fMRI, but I'm not sure if that's because it is listed as the first in the sources or if it is because it has the biggest matrix. Either way, this is causing problems downstream as the reference image for the task fMRI is not the same shape.

What command did you use?

fprodents --notrack --skip-bids-validation -vv --participant_label surface -t visstim --use-syn --force-syn /data/coil_benchmark/data/bids/ /data/coil_benchmark/data/derivatives/fprodents/ participant

What version of the software are you running?

0c497ab

How are you running this software?

Docker

Is your data BIDS valid?

Yes

Are you reusing any previously computed results?

Work directory

Please copy and paste any relevant log output.

No response

Additional information / screenshots

No response

@eilidhmacnicol eilidhmacnicol added the bug Something isn't working label Apr 20, 2023
@eilidhmacnicol
Copy link
Contributor Author

eilidhmacnicol commented Apr 20, 2023

Just seen that this has been discussed elsewhere (#255, #312) I did check before I posted but clearly I didn't go far enough back...

Worth noting that no error was passed with respect to the reference workflow in my case, and it was just the downstream motion correction (using the reference image as the base) that flagged the problem.

@effigies
Copy link
Member

Yes, we should deal with this. I see three options:

  1. Split per-EPI (ENH: Split SyN fieldmap estimates per-EPI #312)
  2. Group by shared parameters: (voxel sizes, TRT, echo time, PEDir)
  3. Group by some shared parameters, use most common voxel size

Pushing me toward (1) is the fact that subjects might move within the scanner. If their head rotates around anything except the PE axis, then SyN will be working with out-of-plane blurs. If we're going to consider within-run head motion a problem to solve (#345), then we should not ignore it between-run.

In the future, we could consider methods to combine fieldmap estimates by shared parameters.

@eilidhmacnicol
Copy link
Contributor Author

Although I was envisioning option 2 when I posted ( rotation between scans during anaesthetised scanning is pretty minimal and the biggest difference is usually a translation caused by field drift), I would also vote for option 1.

In its current draft, fprodents is estimating the distortion for each BOLD file separately when using PE-POLAR field maps, so I like the continuity across methods.

@effigies
Copy link
Member

Okay. I will rebase #312 on master, and we can aim to get it in for 2.5.0. Working on another bug at the moment, so might be tomorrow.

Not to shut down discussion, too quickly. Happy to consider other options, too.

@eilidhmacnicol
Copy link
Contributor Author

Tomorrow is fine; fprodents has a lot of other problems that need addressing simultaneously 😅

@effigies
Copy link
Member

#312 merged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants