-
Notifications
You must be signed in to change notification settings - Fork 133
WIP: PETQC implementation #1399
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
base: master
Are you sure you want to change the base?
Conversation
…-related IQMS and a visual report that plot the FD timeserie across time frames and the rotation and translation parameters in a visual report. We estimate motion correct using 3DVolreg, choosing the frame with the highest intensity sum across voxels as the reference fix: in the end, the hmc workflow is quite different between PET and fMRI, so I returned the HMC workflow implementation for fMRI to its original state
Many thanks, @celprov ! I think we need an update in niworkflows on this, particularly adding some pet information to the nipreps.json file. Will open a separate issue and PR to address this. Unless, @mgxd is faster than me ;-) something like this ' "sub-{subject}/{datatype}/sub-{subject}[_ses-{session}][_acq-{acquisition}][_rec-{reconstruction}][_run-{run}][desc-{desc}]{suffix}{extension<.png|.svg|.html>|.png}" |
Still testing this, and narrowing some issues down to pybids and niworkflows. This is after updating the nipreps.json file in niworkflows, so default_path_patterns include pet.
2025-05-05 10:21:12 | IMPORTANT | mriqc | Extracting metadata and entities for 1 input runs of modality 'pet'... @effigies @mgxd do you mind taking a quick look to move this forward? Many thanks |
Updated nipreps.json with default_path_patterns "default_path_patterns": [ |
Adding "sub-{subject}/{datatype}/sub-{subject}[_ses-{session}][_acq-{acquisition}][_ce-{ceagent}][_rec-{reconstruction}][_run-{run}][_space-{space}][_cohort-{cohort}][desc-{desc}]{suffix}{extension<.png|.svg|.html>|.png}" to the niworkflows nipreps.json file seemed to work out. Now getting File "/Users/martinnorgaard/Dropbox/Mac/Documents/GitHub/mriqc/mriqc/reports/individual.py", line 90, in _single_report
|
(mriqc) $ mriqc /Users/martinnorgaard/Documents/GitHub/mriqc/mriqc/data/pet/ /Users/martinnorgaard/Documents/GitHub/mriqc/mriqc/data/pet/derivatives/mriqc participant --no-subRunning MRIQC version 25.1.0.dev49+g1b92a165.d20250509NOTICE This product includes software developed by Portions of this software were developed at the Department of This software contains code ultimately derived from the This software is also distributed as a Docker container image.
2025-05-09 14:24:38 | IMPORTANT | mriqc | Extracting metadata and entities for 1 input runs of modality 'pet'...
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice! Took a quick look and left some inline comments - otherwise a few things:
- Love that you include test data as we will want to test this, but it is much too big to be included in the repo. Not sure what MRIQC conventions currently are but perhaps it can be in a separate repo or annexed.
- If you have already made the required changes for niworkflows / nireports, you should open a PR for each - happy to review
# Check modality | ||
modality = meta.get('modality', None) or meta.get('suffix', None) or modality | ||
if modality not in ('T1w', 'bold', 'T2w'): | ||
if modality not in ('T1w', 'bold', 'T2w','pet'): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The message below needs updating too
mriqc/qc/pet.py
Outdated
in_file_ref = Path(self.inputs.out_file) | ||
|
||
fname = in_file_ref.name.rstrip("".join(in_file_ref.suffixes)) | ||
out_file = (Path(runtime.cwd) / (f"plot_{fname}_rotations.png")).resolve() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
out_file = (Path(runtime.cwd) / (f"plot_{fname}_rotations.png")).resolve() | |
out_file = Path(f"plot_{fname}_rotations.png").absolute() |
|
||
|
||
def hmc(name='fMRI_HMC', omp_nthreads=None): | ||
def hmc(name='fmriHMC', omp_nthreads=None): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this change needed? Otherwise, we should move all MRIQC workflows to follow nipreps style in a separate PR
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is a mistake from my part, better to revert
return workflow | ||
|
||
|
||
def compute_acf_fwhm(in_file): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this looks like it should be an interface
mriqc/workflows/pet/base.py
Outdated
config.loggers.workflow.info(message) | ||
|
||
# Define workflow, inputs and outputs | ||
# 0. Get data, put it in RAS orientation |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is the data being reoriented?
from mriqc.workflows.pet.output import init_pet_report_wf | ||
|
||
|
||
def pet_qc_workflow(name='petMRIQC'): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
since this is all new, perfect opportunity to start off using the nipreps coding style
def pet_qc_workflow(name='petMRIQC'): | |
def init_pet_qc_wf(name='pet_qc_wf'): |
…reportlets datasink
Need to be copied to the niworkflows package directory under /data/nipreps.json
Draft of the PETQC workflow implementation.
I have modified MRIQC to add PET as an accepted modality type.
The workflow estimates motion correction parameter using 3dVolReg.
I added a node that determines the reference volume as the volume with the highest intensity sum and pass it on to the basefile argument of the VolReg interface.
Then, we generate a visual report that plots the framewise displacement, rotation and translation timeseries.
It also computes the mean FD, num_fd and perc_fd IQMs.
I did not manage to make it work until the end on the example pet data included in this repo
cc @mnoergaard