FIX: Revise the TOPUP workflow and related utilities

[oesteban]

Debugging towards fixing nipreps/fmriprep#2830.

I'm under the impression that the fieldmap estimated by topup is only correct in polarity if the input EPIs are RA{S,I} or AR{S,I} oriented.

The strategy here would be to force all axis to be filled from left to right.

EDIT (new when calling the draft finished):

This PR includes several improvements and changes towards making proper interpretation and reuse of TOPUP outputs:

• Adds code comments at all workflows and functions with substantial edits, trying to help future maintenance.
• Reorients all input images to have positive principal cosines (i.e., axes go L->R, P->A, I->S despite their ordering). The intent is to minimize the possible orientations fed into TOPUP (see 2444c11).
• Rids TOPUP from calculating head motion, it is done outside, with the hope of a full integration with NiTransforms concatenation of transforms (see 2444c11).
• Revises the fix_coeff node to consider the fact that the internal coordinate system of FSL is LAS (see e80e68d). I'm rather positive about the LR flip, since I've seen it on both @mgxd's and @Gilles86's data (e.g., TOPUP's field and warpfieldXX outputs showcase the flip). I'm less positive about the sign change of the field itself (only testable on @Gilles86's data because @mgxd's has j direction for PE).
• Reorders the input EPI runs so that TOPUP always gets the same ordering, preferentially starting with a positive direction encoding (i.e., i, j, or k) - see 72640e7

This does not fully-cover nipreps/fmriprep#2830 (nor #218 EDIT - it does cover this issue, finally), but I have checked that at least the reference image is generated by SDCFlows, and the corresponding by TOPUP are in spatial agreement (i.e., correction is properly applied to the EPIs fed in the estimation) on both datasets (Mathias' and Gilles'). A subsequent PR will address the unwarping of IntendedFor EPI targets.

[codecov-commenter]

Codecov Report

Base: 83.02% // Head: 82.54% // Decreases project coverage by -0.48% ⚠️

Coverage data is based on head (0ab82fe) compared to base (b1c2baf).
Patch coverage: 45.83% of modified lines in pull request are covered.

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #278      +/-   ##
==========================================
- Coverage   83.02%   82.54%   -0.49%     
==========================================
  Files          25       25              
  Lines        1850     1873      +23     
  Branches      278      281       +3     
==========================================
+ Hits         1536     1546      +10     
- Misses        282      295      +13     
  Partials       32       32              

Flag	Coverage Δ
travis	82.54% <45.83%> (-0.49%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
sdcflows/interfaces/utils.py	79.27% <40.90%> (-4.23%)	⬇️
sdcflows/workflows/fit/pepolar.py	100.00% <100.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

[oesteban]

Okay, for the most part, this seems to work on both @Gilles86 and @mgxd datasets - not sure about generalization to new datasets, but this is a start.

There's a remaining issue with @mgxd, being that the registration between the fieldmap reference and the EPI runs is not great. But I think that should be addressed on a separate issue.

Reviews at this point are welcome and important.

cc/ @effigies

[mgxd]

Did a quick pass, still having trouble conceptualizing the whole process. Do you happen to have some screenshots on how these results look on the troublesome datasets before / after this patch?

[oesteban]

Okay, now with tests revised for the new set of reorientation operations

[oesteban]

still having trouble conceptualizing the whole process. Do you happen to have some screenshots on how these results look on the troublesome datasets before / after this patch?

I had @Gilles86's reports - maybe he could confirm whether things are looking better now? In his case (nipreps/fmriprep#2830), the displacements were flipped (i.e., in the opposed direction).

On your data, @mgxd, the diagnosis was similar. However, the results are still a bit off because (I suspect but haven't confirmed) of the registration between the distorted EPI and fieldmap reference.

Still hoping to put together more visual documentation to make it easier to understand both the problem and the solution.

[oesteban]

I'll merge after @eilidhmacnicol's blessings.

[eilidhmacnicol]

LGTM. Left a few comments, but nothing critical to address before merging.

What was the rationale behind the b02b0.cnf changes? I understand the estmov change to stop the estimation of motion (since it is done elsewhere) but what was the reasoning for the reduction of levels to calculate the field?

[oesteban]

What was the rationale behind the b02b0.cnf changes? I understand the estmov change to stop the estimation of motion (since it is done elsewhere) but what was the reasoning for the reduction of levels to calculate the field?

I could have kept the number of levels, but it didn't make much sense as the blips come already aligned. Keeping them was a waste of resources and time for no improvement of the nonlinear component of the estimation (which is done in the latter levels of registration).

[Gilles86]

Would it be possible to send me the `_boldref.nii.gz`-files?

…

On Mon, 19 Sept 2022, 21:02 Oscar Esteban, ***@***.***> wrote: still having trouble conceptualizing the whole process. Do you happen to have some screenshots on how these results look on the troublesome datasets before / after this patch? I had @Gilles86 <https://github.com/Gilles86>'s reports - maybe he could confirm whether things are looking better now? In his case ( nipreps/fmriprep#2830 <nipreps/fmriprep#2830>), the displacements were flipped (i.e., in the opposed direction). On your data, @mgxd <https://github.com/mgxd>, the diagnosis was similar. However, the results are still a bit off because (I suspect but haven't confirmed) of the registration between the distorted EPI and fieldmap reference. Still hoping to put together more visual documentation to make it easier to understand both the problem and the solution. — Reply to this email directly, view it on GitHub <#278 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAIPRTVEAHGZF7MJZ3BT7D3V7C2ERANCNFSM6AAAAAAQDNFDKE> . You are receiving this because you were mentioned.Message ID: ***@***.***>