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

✨Feature/update pixdim4 for the final *bold outputs #2154

Open
wants to merge 12 commits into
base: develop
Choose a base branch
from
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

- `pyproject.toml` file with `[build-system]` defined.
- [![pre-commit.ci status](https://results.pre-commit.ci/badge/github/FCP-INDI/C-PAC/main.svg)](https://results.pre-commit.ci/latest/github/FCP-INDI/C-PAC/main) badge to [`README`](./README.md).
- validation node to match the pixdim4 of CPAC processed bold outputs with the original raw bold sources.

### Changed

Expand Down
41 changes: 39 additions & 2 deletions CPAC/pipeline/engine.py
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,12 @@
from CPAC.pipeline import nipype_pipeline_engine as pe
from CPAC.pipeline.check_outputs import ExpectedOutputs
from CPAC.pipeline.nodeblock import NodeBlockFunction
from CPAC.pipeline.utils import MOVEMENT_FILTER_KEYS, name_fork, source_set
from CPAC.pipeline.utils import (
MOVEMENT_FILTER_KEYS,
name_fork,
source_set,
validate_outputs,
)
from CPAC.registration.registration import transform_derivative
from CPAC.resources.templates.lookup_table import lookup_identifier
from CPAC.utils.bids_utils import res_in_filename
Expand Down Expand Up @@ -1371,6 +1376,7 @@ def gather_pipes(self, wf, cfg, all=False, add_incl=None, add_excl=None):
write_json.inputs.json_data = json_info

wf.connect(id_string, "out_filename", write_json, "filename")

ds = pe.Node(DataSink(), name=f"sinker_{resource_idx}_{pipe_x}")
ds.inputs.parameterization = False
ds.inputs.base_directory = out_dct["out_dir"]
Expand All @@ -1394,7 +1400,38 @@ def gather_pipes(self, wf, cfg, all=False, add_incl=None, add_excl=None):
subdir=out_dct["subdir"],
),
)
wf.connect(nii_name, "out_file", ds, f'{out_dct["subdir"]}.@data')
if resource.endswith("_bold"):
# Node to validate TR (and other scan parameters)
validate_bold_header = pe.Node(
Function(
input_names=["input_bold", "RawSource_bold"],
output_names=["output_bold"],
function=validate_outputs,
imports=[
"from CPAC.pipeline.utils import find_pixdim4, update_pixdim4"
],
),
name=f"validate_bold_header_{resource_idx}_{pipe_x}",
)
raw_source, raw_out = self.get_data("bold")
wf.connect(
[
(nii_name, validate_bold_header, [(out, "input_bold")]),
(
raw_source,
validate_bold_header,
[(raw_out, "RawSource_bold")],
),
(
validate_bold_header,
ds,
[("output_bold", f'{out_dct["subdir"]}.@data')],
),
]
)
else:
wf.connect(nii_name, "out_file", ds, f'{out_dct["subdir"]}.@data')

wf.connect(write_json, "json_file", ds, f'{out_dct["subdir"]}.@json')
outputs_logger.info(expected_outputs)

Expand Down
130 changes: 130 additions & 0 deletions CPAC/pipeline/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,13 +17,143 @@
"""C-PAC pipeline engine utilities."""

from itertools import chain
import os
import subprocess

import nibabel as nib

from CPAC.func_preproc.func_motion import motion_estimate_filter
from CPAC.utils.bids_utils import insert_entity
from CPAC.utils.monitoring import IFLOGGER

MOVEMENT_FILTER_KEYS = motion_estimate_filter.outputs


def find_pixdim4(file_path):
"""Find the pixdim4 value of a NIfTI file.

Parameters
----------
file_path : str
Path to the NIfTI file.

Returns
-------
float
The pixdim4 value of the NIfTI file.

Raises
------
FileNotFoundError
If the file does not exist.
nibabel.filebasedimages.ImageFileError
If there is an error loading the NIfTI file.
IndexError
If pixdim4 is not found in the header.
"""
if not os.path.isfile(file_path):
error_message = f"File not found: {file_path}"
raise FileNotFoundError(file_path)

try:
nii = nib.load(file_path)
header = nii.header
pixdim = header.get_zooms()
return pixdim[3]
except nib.filebasedimages.ImageFileError as e:
error_message = f"Error loading the NIfTI file: {e}"
raise nib.filebasedimages.ImageFileError(error_message)
except IndexError as e:
error_message = f"pixdim4 not found in the header: {e}"
raise IndexError(error_message)


def update_pixdim4(file_path, new_pixdim4):
"""Update the pixdim4 value of a NIfTI file using 3drefit.
Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I am not sure about this one. May be @sgiavasis could advise on this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The reason to use AFNI 3drefit instead of nibabel is to change the Time step in 3dinfo as well along with pixdim4


Parameters
----------
file_path : str
Path to the NIfTI file.
new_pixdim4 : float
New pixdim4 value to update the NIfTI file with.

Raises
------
FileNotFoundError
If the file does not exist.
subprocess.CalledProcessError
If there is an error running the subprocess.

Notes
-----
The pixdim4 value is the Repetition Time (TR) of the NIfTI file.

"""
if not os.path.isfile(file_path):
error_message = f"File not found: {file_path}"
raise FileNotFoundError(error_message)

# Print the current pixdim4 value for verification
IFLOGGER.info(f"Updating {file_path} with new pixdim[4] value: {new_pixdim4}")

# Construct the command to update the pixdim4 value using 3drefit
command = ["3drefit", "-TR", str(new_pixdim4), file_path]

try:
subprocess.run(command, check=True)
IFLOGGER.info(f"Successfully updated TR to {new_pixdim4} seconds.")
except subprocess.CalledProcessError as e:
error_message = f"Error occurred while updating the file: {e}"
raise subprocess.CalledProcessError(error_message)


def validate_outputs(input_bold, RawSource_bold):
"""Match pixdim4/TR of the input_bold with RawSource_bold.

Parameters
----------
input_bold : str
Path to the input BOLD file.
RawSource_bold : str
Path to the RawSource BOLD file.

Returns
-------
output_bold : str
Path to the output BOLD file.

Raises
------
Exception
If there is an error in finding or updating pixdim4.
"""
try:
output_bold = input_bold
output_pixdim4 = find_pixdim4(output_bold)
source_pixdim4 = find_pixdim4(RawSource_bold)

if output_pixdim4 != source_pixdim4:
IFLOGGER.info(
"TR mismatch detected between output_bold and RawSource_bold."
)
IFLOGGER.info(f"output_bold TR: {output_pixdim4} seconds")
IFLOGGER.info(f"RawSource_bold TR: {source_pixdim4} seconds")
IFLOGGER.info(
"Attempting to update the TR of output_bold to match RawSource_bold."
)
update_pixdim4(output_bold, source_pixdim4)
else:
IFLOGGER.debug("TR match detected between output_bold and RawSource_bold.")
IFLOGGER.debug(f"output_bold TR: {output_pixdim4} seconds")
IFLOGGER.debug(f"RawSource_bold TR: {source_pixdim4} seconds")
return output_bold
except Exception as e:
error_message = f"Error in validating outputs: {e}"
IFLOGGER.error(error_message)
return output_bold
Comment on lines +152 to +154
Copy link
Member

Choose a reason for hiding this comment

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

Do we want to raise an exception here instead of (log an error message and return the path to the bold file)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch, I am not sure if we want to totally crash (by raising exception) CPAC here or if we only say following resources were not validated but still its here?

Copy link
Contributor Author

@birajstha birajstha Sep 18, 2024

Choose a reason for hiding this comment

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

or probably add a swtich to the config to allow choosing between

  • raising exception
  • continue



def name_fork(resource_idx, cfg, json_info, out_dct):
"""Create and insert entities for forkpoints.

Expand Down
Loading