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

Load T1w-to-standard transform to same space as volumetric BOLD scan #926

Merged
merged 42 commits into from
Sep 25, 2023

Conversation

tsalo
Copy link
Member

@tsalo tsalo commented Jul 11, 2023

Closes #925.

Changes proposed in this pull request

  • Add an inverted flag to get_std2bold_xfms to get transforms to MNI152NLin6Asym instead of from it.
  • Add steps in init_execsummary_functional_plots_wf to warp the mean BOLD image and the BOLD reference image to MNI152NLin6Asym (which is the space the T1w/T2w should be in) before plotting them.
  • Make MNI152NLin6Asym the preferred space for nibabies data.
  • Use the template from the anat-to-template transform to select the appropriate NIfTI preprocessed BOLD and boldref for cifti runs with the executive summary enabled.
    • This means we need to ensure that there are BOLD files available in the target space, which we now do.

Documentation that should be reviewed

@tsalo tsalo changed the title Use MNI152NLin6Asym for fsLR no matter what Warp BOLD images to MNI152NLin6Asym for executive summary overlays Jul 14, 2023
@tsalo tsalo added the bug Issues noting problems and PRs fixing those problems. label Jul 14, 2023
@codecov
Copy link

codecov bot commented Jul 14, 2023

Codecov Report

Attention: 8 lines in your changes are missing coverage. Please review.

Files Coverage Δ
xcp_d/workflows/anatomical.py 91.30% <100.00%> (+0.14%) ⬆️
xcp_d/workflows/base.py 97.12% <ø> (ø)
xcp_d/utils/bids.py 86.62% <78.94%> (-0.58%) ⬇️
xcp_d/workflows/execsummary.py 95.08% <83.33%> (-0.41%) ⬇️

... and 1 file with indirect coverage changes

📢 Thoughts on this report? Let us know!.

@tsalo
Copy link
Member Author

tsalo commented Jul 17, 2023

@madisoth and @lundq163, could one of you pull the branch associated with this PR and see if it fixes the issue?

@tsalo
Copy link
Member Author

tsalo commented Jul 20, 2023

Per @lundq163, the problem is still present with the current changes.

@lundq163
Copy link

lundq163 commented Jul 21, 2023

hi @tsalo, i just retested it again and am seeing the same issue as before. here are the paths for the output and work dirs on MSI:
/scratch.global/lundq163/mcribs_test/BCP_man_corr_asegs/derivatives_xcpd_0720
/scratch.global/lundq163/mcribs_test/BCP_man_corr_asegs/work_xcpd_0720

and this was my run command:

singularity run --cleanenv \
    -B /home/faird/shared/code/external/utilities/freesurfer_license/license.txt:/opt/freesurfer/license.txt \
    -B /scratch.global/lundq163/mcribs_test/BCP_man_corr_asegs/derivatives_goodvox_mcribs_0629_tm:/data:ro \
    -B /scratch.global/lundq163/mcribs_test/BCP_man_corr_asegs/derivatives_xcpd_0720:/out \
    -B /scratch.global/lundq163/mcribs_test/BCP_man_corr_asegs/work_xcpd_0720:/work \
    -B /home/faird/shared/code/external/pipelines/xcp_d_test_binds/20230712_fix-collection/xcp_d/xcp_d:/usr/local/miniconda/lib/python3.8/site-packages/xcp_d \
    /home/faird/shared/code/external/pipelines/xcp_d/xcp_d_unstable_07172023a.sif /data /out participant \
    --input-type nibabies \
    --cifti \
    --despike \
    --resource-monitor \
    --dcan-qc \
    -w /work \
    --omp-nthreads 3 \
    --warp-surfaces-native2std \
    -f 0.3 \
    -m \
    -vv \
    --participant-label 981854 \

@madisoth and I took some time to look through the work dir, and he concluded that the command being used to warp the mean BOLD looks correct (<...>/cifti_postprocess_<n>_wf/execsummary_functional_plots_wf/warp_mean_bold_to_mni/command.txt) but the result looks the the opposite of what it should be: if transforming from MNIInfant to MNI adult space the brain should be getting bigger, not smaller. Thomas suggested maybe one or both of the transforms (MNIInfant -> MNI2009, MNI2009 -> MNI6) are misnamed and it's actually the inverse transform, or something along those lines? I will try to look into it more on my own time next week too. Lmk if you need me to do anymore testing though moving forward, thanks!

@tsalo
Copy link
Member Author

tsalo commented Jul 27, 2023

Thanks @lundq163! I'll take a look today. My guess is that I messed up the inverted version of the transform collection function.

@tsalo
Copy link
Member Author

tsalo commented Jul 28, 2023

I can't see anything wrong with the ApplyTransforms calls or the collection of the transforms, so I worry that maybe the MNIInfant-to-MNI152NLin2009cAsym transform is actually a copy of the MNI152NLin2009cAsym-to-MNIInfant transform.

@tsalo tsalo added the dcan DCAN-related issues and PRs label Jul 31, 2023
@tsalo
Copy link
Member Author

tsalo commented Aug 2, 2023

@lundq163 do you think you could give this branch another shot? Hopefully I fixed the problem.

Comment on lines 34 to 38
"nifti": [
"MNIInfant",
"MNI152NLin6Asym",
"MNIInfant",
"MNI152NLin2009cAsym",
],
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Make MNI152NLin6Asym the preferred space for nibabies data.

tsalo added 3 commits August 25, 2023 10:44
This should ensure that BOLD and anatomical NIfTIs are in the same space if the cifti flag is used.
@tsalo
Copy link
Member Author

tsalo commented Aug 25, 2023

I believe the problem is just that the space selected for NIfTI BOLD and boldref files was independent from the space selected for the anat-to-template transform, which is what is applied to the T1w data.

ds001419_cifti has a T1w-to-MNI152NLin6Asym transform, but that's only because of AROMA. There are no MNI152NLin6Asym BOLD outputs, except smoothAROMAnonaggr files. Somehow I need to take the run data into account when selecting the target space across runs, which is real fun.

@tsalo
Copy link
Member Author

tsalo commented Aug 25, 2023

I am now looping through allowed volumetric spaces for a given input type in order to find one with NIfTI BOLD files in that space when running with the --cifti flag. This should ensure that the target space for the anatomical images is the same as the space the preprocessed BOLD NIfTIs are in, for the sake of the T1w-on-BOLD and related figures. This somewhat reverts a subset of changes in #759, where I hardcoded MNI152NLin6Asym as the target space for T1ws when running with --cifti.

I am concerned that the brainsprite overlaying the white matter and pial surfaces on top of the anatomicals won't work correctly if the anatomicals are in a space other than MNI152NLin6Asym. In the case of ds001419-cifti, the best available space is MNI152NLin2009cAsym, so I will compare the brainsprite from this PR's ds001419-cifti executive summary against the one from main, I guess.

@tsalo
Copy link
Member Author

tsalo commented Sep 12, 2023

@madisoth @lundq163 would you mind checking the new version of this branch?

@tsalo tsalo changed the title Warp BOLD images to MNI152NLin6Asym for executive summary overlays Load T1w-to-standard transform to same space as volumetric BOLD scan Sep 12, 2023
@tsalo tsalo merged commit 778c96f into PennLINC:main Sep 25, 2023
3 of 4 checks passed
@tsalo tsalo deleted the fix-collection branch September 25, 2023 19:11
@tsalo tsalo restored the fix-collection branch October 9, 2023 20:31
@tsalo tsalo deleted the fix-collection branch November 2, 2023 17:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Issues noting problems and PRs fixing those problems. dcan DCAN-related issues and PRs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

CIFTI run with Nibabies data collected MNIInfant for BOLD and MNI152NLin6Asym for T1w
2 participants