Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

FIX: registration issue between T1w and BOLD in specific datasets #2607

Closed

Conversation

HippocampusGirl
Copy link
Contributor

Changes proposed in this pull request

  • Switch to using mri_coreg in fsl_bbr_wf instead of flirt as suggested by @effigies and @oesteban in Failed registrations between BOLD and T1w #2591
  • Add -basescale 1 parameter to the flirt boundary-based registration call to avoid the scaling behavior there too. As far as I can tell, this does not have side effects, but it may make sense to wait for a response from the flirt developers before merging this one.

Documentation that should be reviewed

No changes to documentation

- 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)
@HippocampusGirl HippocampusGirl changed the title FIX: registration issue between T1w and BOLD FIX: registration issue between T1w and BOLD in specific datasets Oct 19, 2021
@oesteban
Copy link
Member

How does this work with your data @HippocampusGirl ?

@effigies
Copy link
Member

Hmm. If we're adding -basescale 1 to the BBR step, then it feels like adding -basescale 1 to the initial FLIRT step is less intrusive than switching to mri_coreg.

Looking at the source:

void set_basescale(const string& filenameA, const string& filenameB)
{
  if (!(gOptions->force_basescale)) { 
    // only try to automatically determine if it was not requested by the user
    volume<float> volA, volB;
    read_volume_hdr_only(volA,filenameA);
    read_volume_hdr_only(volB,filenameB);
    float maxdimA = Max(Max(volA.xdim(),volA.ydim()),volA.zdim());
    float maxdimB = Max(Max(volB.xdim(),volB.ydim()),volB.zdim());
    if (Max(maxdimA,maxdimB)>12) {  // over 150% of largest (8mm) scale
      gOptions->basescale = Max(maxdimA,maxdimB)/8;
    }
    float mindimA = Min(Min(volA.xdim(),volA.ydim()),volA.zdim());
    float mindimB = Min(Min(volB.xdim(),volB.ydim()),volB.zdim());
    if (Min(mindimA,mindimB)<0.75) { // between 0.5 and 1 mm 
      gOptions->basescale = Min(mindimA,mindimB);
    }
  }
}

-basescale 1 only has an effect if a voxel dimension is > 12mm or <0.75mm, so I think we can be reasonably confident that the number of datasets affected will be quite small.

I'm okay moving to mri_coreg if everyone wants to keep on that way, but I think I have a slight preference for just setting -basescale 1 on both steps of the FSL pipeline for now, and then moving to a common pipeline in future versions.

Sorry for jumping one way and then the other.

@HippocampusGirl
Copy link
Contributor Author

@oesteban
I have only tested this with single subjects so far, where the results are very promising. I will be re-running some larger datasets that had the issue over the next few days, and will get back to you about that.

@effigies
I am happy either way, but I would prefer the mri_coreg option, since it would make the workflows more consistent between running with and without --fs-no-reconall.

HippocampusGirl added a commit to HALFpipe/HALFpipe that referenced this pull request Oct 19, 2021
@oesteban
Copy link
Member

@HippocampusGirl thanks much for the update and the effort.

Since setting -basescale 1 on both FLIRT processes seems trivial, why don't we change the base of this PR to master and add the parameter to maint/20.2.x as @effigies suggests? That's probably going to be the best call to minimize the workload of the LTS maintainers.

Would that be okay?

@mgxd
Copy link
Collaborator

mgxd commented Dec 15, 2021

can this be closed now that #2624 and #2625 are in?

@effigies effigies closed this Dec 15, 2021
HippocampusGirl added a commit to HALFpipe/HALFpipe that referenced this pull request Jan 12, 2022
- Includes only the `-basescale 1` fix and not the move to `mri_coreg`
  as discussed here nipreps/fmriprep#2607 (comment)
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants