Update conf2hires

[takuya-hayashi]

bbregister should not need initialization, since T2w is already initialized with FS BBR in PreFreeSurferPipeline

[glasserm]

The way this was done previously was with initializing to an identity matrix; however, perhaps using the headers is safer if FreeSurfer behaves correctly.

[takuya-hayashi]

I confirmed it worked better than using --init-coreg in our hires data. Probably it behaves correctly. 2023年5月13日(土) 8:49 glasserm ***@***.***>:

…

The way this was done previously was with initializing to an identity matrix; however, perhaps using the headers is safer if FreeSurfer behaves correctly. — Reply to this email directly, view it on GitHub <#265 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AD57Y5TPUMP6WSL3SUT6SVTXF3D67ANCNFSM6AAAAAAYADCY7A> . You are receiving this because you authored the thread.Message ID: ***@***.***>

[coalsont]

I don't see any issues with the edit, though I am not too familiar with freesurfer conventions. Should we merge it?

[glasserm]

@mharms should let us know what he thinks. conf2hires was not written by us I think.

[coalsont]

I guess there is also the question of the logic:

HCPpipelines/FreeSurfer/FreeSurferPipeline.sh

Line 47 in 005a096

if [[ "${which_recon_all}" = "" || "${which_conf2hires}" == "" || "${which_longmc}" = "" ]] ; then

If $PATH somehow already contains these, the ones in the pipelines won't get used. How many of these exist in a vanilla freesurfer 6 install?

[mharms]

What is the motivation/necessity for this proposed change to the bbregister call in conf2hires? If it ain't broke, don't fix it...

[takuya-hayashi]

We have encountered hire-res data of which T2wtoT1w registration was not aligned very well due to initialization of bbregister. HCP pipeline does the initialization using FSL BBR in PreFresurfer, however, bbregister in the conf2hire currently runs the initialization again with 'mri_coreg', which is not needed anymore in HCP pipeline. Using an option of --init-header does not do any initialization in the bbregister then BBR likely works better. This approach is the same as in FreeSurferPipeline-v5.3.sh.

[mharms]

While seemingly a simple change, I'm concerned about unintended consequences here. Also, "probably it behaves correctly" and "likely works better" are not particularly reassuring regarding my unintended consequences concern.

[glasserm]

This use of mri_coreg was introduced by whomever wrote conf2hires. My original implementation in FS5.3 used the identity matrix as the initialization. If we are initializing by FSL BBR, mri_coreg is likely to worsen the initialization if it does anything at all.

[coalsont]

--init-coreg goes back to the first version of fs6 conf2hires committed to the repo, message "FreeSurfer 6 customizations from Doug Greve - V5":

944f0af#diff-a22b98e4fe1127dcca7f2ffd5af60a5005fd09cc7daa07ce892d471c9c293232R278

So, Matt's edits to the 5.3.0-HCP stream probably didn't make it into the V6 adaptation. NHP is probably just a more challenging application, and exposed that our provided initial coregistration was being disregarded in favor of the freesurfer default (which doesn't expect an existing coregistration of the inputs).

It would be nice to know whether --init-header actually means the same thing as a nifti mm-space identity transform, but it would be hard to imagine another interpretation, particularly if the headers of the two images are identical (because we already resampled both to the template space).

[takuya-hayashi]

Using identify matrix is better to exactly follow the 5.3.0-HCP and avoid confusion?

[coalsont]

Assuming that --init-header does the same thing as an identity matrix for the inputs we give to freesurfer, I think I prefer the --init-header way, as it avoids needing to specify a transform file (when all we want is the identity). This would avoid the ugliness of creating a transform file, or relying on the user not having overwritten the identity matrix in their freesurfer install, etc.

[glasserm]

Assuming FreeSurfer behaves sensibly, I think init-header would be more robust also. In the FreeSurfer 5.3 case, I don't know if that option was available.

[takuya-hayashi]

Thanks. I'd know how @mharms thinks about this, in particular, if it is okay to edit the code of others. I'd support to increase the functionality of FreeSurfer for NHP studies, because FS is in principle so robust and realiable. I also hope a part of which is or will be useful for human studies as well. If needed, we can confirm the effect of edits in a large human dataset.

[coalsont]

conf2hires is in the pipelines because we want to use an edited version, not the related steps that are somewhere inside freesurfer. The purpose of it is for us to edit it for our needs. I think @mharms is mainly worried about not knowing what this edit would have done on subjects we have run the prior version on.

[mharms]

Yes, perhaps -init-header is a better technical choice for the HCPpipelines, but to my knowledge, this would be the first change in a while that would result in non-identical structural results. So, for example, if we make this change, we probably should not use any subsequent releases for the processing of AABC cross-sectional structural data.

[glasserm]

This should not have any effect if FS BBR is already working properly.

[mharms]

In that case, why change it? :)

[glasserm]

Because FS BBR is not working in all usecases. It is working for the AABC usecase.

[coalsont]

This edit brings the FS 6 stream closer to the edits deliberately made to the 5.3.0-HCP stream (and avoids destroying the epi_reg that at least puts some qa messages into a text file to look at the result), which seems like a good thing to me. The point of master is to take in improvements. If the theory of "FS BBR produces indistinguishable results as long as the initialization isn't bad" isn't persuasive enough, then AABC (and any other projects where it might matter) can stick with an older release for yet-to-be-processed data, that is why the releases exist. This concern shouldn't prevent us from making improvements to the development branch.

[takuya-hayashi]

I'd note that "--init-header" method only works for structure but not for function and diffuison MRI preprocessing.

[coalsont]

But conf2hires doesn't get used on anything other than structural, right?

[takuya-hayashi]

Right. conf2hires should be fine. I was just curious why func and diff did not work with --init-header.