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: Replace initial FLIRT with mri_coreg, use -basescale 1 for FLIRT-BBR #2625

Merged
merged 4 commits into from
Dec 8, 2021

Conversation

effigies
Copy link
Member

Rebases #2607 on master.

Fast-tracked to get this into the next 21.0.0 RC. Need to figure out numpy failures in the docker image to patch 20.2.x.

- Avoid the issues with high-resolution anatomical data explained in
  nipreps#2591
- Improve consistency between running with `--fs-no-reconall` and
  without
- Disables `flirt` scaling of input transformation matrices, which may
  lead to issues with high-resolution T1w data (see nipreps#2591)
@effigies
Copy link
Member Author

@mgxd We need to update the reports and boilerplate to indicate that we're using mri_coreg. Not sure it's worth doing before an RC. Let me know if I should work on this or start a new PR.

@mgxd
Copy link
Collaborator

mgxd commented Nov 12, 2021

Once we get niworkflows passing, I'll kick off the release chain, but I don't expect we'll get to fmriprep until Monday. Depending on how long you think those updates will take, you can probably just do it here.

@effigies
Copy link
Member Author

Ah, niworkflows is going to be at least two hours to finish the mask regression tests. I'll see what I can get to by 5.

@effigies
Copy link
Member Author

Not really sure how to fix this in the citation boilerplate (from artifacts:

Functional data preprocessing

: For each of the 1 BOLD runs found per subject (across all
tasks and sessions), the following preprocessing was performed.

Preprocessing of B0 inhomogeneity mappings

: A total of 1 fieldmaps were found available within the input
BIDS structure for this particular subject.
A deformation field to correct for susceptibility distortions was estimated
based on *fMRIPrep*'s *fieldmap-less* approach.
The deformation field is that resulting from co-registering the EPI reference
to the same-subject T1w-reference with its intensity inverted [@fieldmapless1;
@fieldmapless2].
Registration is performed with `antsRegistration`
(ANTs 2.3.3), and
the process regularized by constraining deformation to be nonzero only
along the phase-encoding direction, and modulated with an average fieldmap
template [@fieldmapless3].
First, a reference volume and its skull-stripped version were generated
from the shortest echo of the BOLD run using a custom
methodology of *fMRIPrep*.
[...]

Pretty sure it should be

Preprocessing of B0 inhomogeneity mappings

: A total of 1 fieldmaps were found available within the input
BIDS structure for this particular subject.
A deformation field to correct for susceptibility distortions was estimated
based on *fMRIPrep*'s *fieldmap-less* approach.
The deformation field is that resulting from co-registering the EPI reference
to the same-subject T1w-reference with its intensity inverted [@fieldmapless1;
@fieldmapless2].
Registration is performed with `antsRegistration`
(ANTs 2.3.3), and
the process regularized by constraining deformation to be nonzero only
along the phase-encoding direction, and modulated with an average fieldmap
template [@fieldmapless3].

Functional data preprocessing

: For each of the 1 BOLD runs found per subject (across all
tasks and sessions), the following preprocessing was performed.
First, a reference volume and its skull-stripped version were generated
from the shortest echo of the BOLD run using a custom
methodology of *fMRIPrep*.
[...]

@codecov-commenter
Copy link

codecov-commenter commented Nov 13, 2021

Codecov Report

Merging #2625 (c52206f) into master (d2ad203) will decrease coverage by 0.02%.
The diff coverage is 20.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2625      +/-   ##
==========================================
- Coverage   45.46%   45.43%   -0.03%     
==========================================
  Files          41       41              
  Lines        3161     3163       +2     
==========================================
  Hits         1437     1437              
- Misses       1724     1726       +2     
Impacted Files Coverage Δ
fmriprep/workflows/bold/registration.py 8.88% <20.00%> (-0.10%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d2ad203...c52206f. Read the comment docs.

@effigies
Copy link
Member Author

@oesteban As the master of the literate workflows, your advice on how to rearrange the boilerplate text would be appreciated.

@mgxd
Copy link
Collaborator

mgxd commented Nov 15, 2021

I was looking into this earlier - from my understanding, order largely depends on the graph order, though __predesc__, __desc__, and __postdesc__ can somewhat help. LiterateWorkflow likely needs a change to allow more fine control within these sections.

@oesteban
Copy link
Member

@oesteban As the master of the literate workflows, your advice on how to rearrange the boilerplate text would be appreciated.

I'm not sure this problem has been introduced here - we probably should get this merged and keep investigating why the boilerplate is garbled.

order largely depends on the graph order, though __predesc__, __desc__, and __postdesc__ can somewhat help. LiterateWorkflow likely needs a change to allow more fine control within these sections.

Agreed.

@effigies
Copy link
Member Author

No it wasn't introduced here. Was just interested in fixing it. I'm okay with merging this as is.

@effigies effigies merged commit ec84230 into nipreps:master Dec 8, 2021
@effigies effigies deleted the enh/mricoreg_flirt branch December 8, 2021 15:05
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.

5 participants