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

➕ Reorient resources to set orientation in config #2140

Merged
merged 39 commits into from
Oct 14, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
39 commits
Select commit Hold shift + click to select a range
b9867c9
:heavy_plus_sign: orientation checks for resources added to PipeConfigs
Aug 16, 2024
81869e2
Update CPAC/pipeline/utils.py
birajstha Aug 23, 2024
0a32a32
Update CPAC/pipeline/engine.py
birajstha Aug 23, 2024
d005b01
Update CPAC/pipeline/engine.py
birajstha Aug 23, 2024
d31b1d9
Update CPAC/pipeline/engine.py
birajstha Aug 23, 2024
5480678
Update CPAC/pipeline/engine.py
birajstha Aug 23, 2024
b965952
Update CPAC/pipeline/engine.py
birajstha Aug 23, 2024
9df6ab5
Update CPAC/pipeline/utils.py
birajstha Aug 23, 2024
c0006cd
Update CPAC/pipeline/utils.py
birajstha Aug 23, 2024
edf6d99
Update CPAC/pipeline/utils.py
birajstha Aug 23, 2024
57d6133
Update CPAC/pipeline/utils.py
birajstha Aug 23, 2024
0f190d8
Update CPAC/pipeline/utils.py
birajstha Aug 23, 2024
c5b4a1e
adding desired-orientation as a config key
Sep 9, 2024
a04d47d
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Sep 9, 2024
f2be81a
reorienting all files before rpool initialization
birajstha Sep 9, 2024
995163c
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Sep 9, 2024
5aeebbb
moved orientation check and reorient if necessary to the wf
birajstha Sep 17, 2024
e8baa28
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Sep 17, 2024
ef10e36
renaming output name correctly
birajstha Sep 17, 2024
6bfd680
handle for read-only template files
birajstha Sep 18, 2024
e454cec
adding reorient nodes in anat, func, freesurfer and template ingress …
birajstha Sep 26, 2024
d47defb
fix a typo in orientation flag for resample
birajstha Oct 1, 2024
c9b69b9
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Oct 1, 2024
d0a7c2a
updating the orientation flag at other places where resolve_resolutio…
birajstha Oct 1, 2024
7246e44
reverting adding reorient node at the very begining and passing orien…
birajstha Oct 1, 2024
f3f3f98
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Oct 1, 2024
1a43750
empty
birajstha Oct 1, 2024
939c7a5
updated tests
birajstha Oct 2, 2024
9df743c
updated the changelog
birajstha Oct 2, 2024
079b1f0
replaced RPI to take in config value in few more places
birajstha Oct 2, 2024
35ed398
adding cfg as parameter in lesion_preproc
birajstha Oct 3, 2024
b368083
sending only orientation not all cfg
birajstha Oct 3, 2024
71dd070
fixed positions of args in lesion_preproc and added default RPI
birajstha Oct 3, 2024
3f1e109
Update CHANGELOG.md
birajstha Oct 9, 2024
d24fae6
Update CHANGELOG.md
birajstha Oct 9, 2024
408d871
Update CHANGELOG.md
birajstha Oct 10, 2024
2895e8c
fixing pre-commit suggestions
birajstha Oct 10, 2024
5eedfb7
Merge branch 'develop' into feature/check_orientations
birajstha Oct 10, 2024
312ef00
Merge branch 'develop' into feature/check_orientations
sgiavasis Oct 14, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -20,11 +20,16 @@ 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).
- `desired_orientation` key in participant-level pipeline config under `pipeline_setup`.
- Required positional parameter "wf" in input and output of `ingress_pipeconfig_paths` function, where a node to reorient templates is added to the `wf`.
- Required positional parameter "orientation" to `resolve_resolution`.
- Optional positional argument "cfg" to `create_lesion_preproc`.

### Changed

- Moved `pygraphviz` from requirements to `graphviz` optional dependencies group.
- Automatically tag untagged `subject_id` and `unique_id` as `!!str` when loading data config files.
- Made orientation configurable (was hard-coded as "RPI").

### Fixed

Expand Down
8 changes: 4 additions & 4 deletions CPAC/anat_preproc/anat_preproc.py
Original file line number Diff line number Diff line change
Expand Up @@ -1233,7 +1233,7 @@ def freesurfer_fsl_brain_connector(wf, cfg, strat_pool, pipe_num, opt):
mem_gb=0,
mem_x=(0.0115, "in_file", "t"),
)
reorient_fs_brainmask.inputs.orientation = "RPI"
reorient_fs_brainmask.inputs.orientation = cfg.pipeline_setup["desired_orientation"]
reorient_fs_brainmask.inputs.outputtype = "NIFTI_GZ"

wf.connect(
Expand All @@ -1255,7 +1255,7 @@ def freesurfer_fsl_brain_connector(wf, cfg, strat_pool, pipe_num, opt):
mem_gb=0,
mem_x=(0.0115, "in_file", "t"),
)
reorient_fs_T1.inputs.orientation = "RPI"
reorient_fs_T1.inputs.orientation = cfg.pipeline_setup["desired_orientation"]
reorient_fs_T1.inputs.outputtype = "NIFTI_GZ"

wf.connect(convert_fs_T1_to_nifti, "out_file", reorient_fs_T1, "in_file")
Expand Down Expand Up @@ -1460,7 +1460,7 @@ def anatomical_init(wf, cfg, strat_pool, pipe_num, opt=None):
mem_gb=0,
mem_x=(0.0115, "in_file", "t"),
)
anat_reorient.inputs.orientation = "RPI"
anat_reorient.inputs.orientation = cfg.pipeline_setup["desired_orientation"]
anat_reorient.inputs.outputtype = "NIFTI_GZ"

wf.connect(anat_deoblique, "out_file", anat_reorient, "in_file")
Expand Down Expand Up @@ -2268,7 +2268,7 @@ def anatomical_init_T2(wf, cfg, strat_pool, pipe_num, opt=None):
mem_gb=0,
mem_x=(0.0115, "in_file", "t"),
)
T2_reorient.inputs.orientation = "RPI"
T2_reorient.inputs.orientation = cfg.pipeline_setup["desired_orientation"]
T2_reorient.inputs.outputtype = "NIFTI_GZ"

wf.connect(T2_deoblique, "out_file", T2_reorient, "in_file")
Expand Down
6 changes: 4 additions & 2 deletions CPAC/anat_preproc/lesion_preproc.py
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ def inverse_lesion(lesion_path):
return lesion_out


def create_lesion_preproc(wf_name="lesion_preproc"):
def create_lesion_preproc(cfg=None, wf_name="lesion_preproc"):
"""Process lesions masks.

Lesion mask file is deobliqued and reoriented in the same way as the T1 in
Expand Down Expand Up @@ -133,7 +133,9 @@ def create_lesion_preproc(wf_name="lesion_preproc"):
mem_x=(0.0115, "in_file", "t"),
)

lesion_reorient.inputs.orientation = "RPI"
lesion_reorient.inputs.orientation = (
cfg.pipeline_setup["desired_orientation"] if cfg else "RPI"
)
lesion_reorient.inputs.outputtype = "NIFTI_GZ"

preproc.connect(lesion_deoblique, "out_file", lesion_reorient, "in_file")
Expand Down
4 changes: 2 additions & 2 deletions CPAC/func_preproc/func_preproc.py
Original file line number Diff line number Diff line change
Expand Up @@ -528,7 +528,7 @@ def func_reorient(wf, cfg, strat_pool, pipe_num, opt=None):
mem_x=(0.0115, "in_file", "t"),
)

func_reorient.inputs.orientation = "RPI"
func_reorient.inputs.orientation = cfg.pipeline_setup["desired_orientation"]
func_reorient.inputs.outputtype = "NIFTI_GZ"

wf.connect(func_deoblique, "out_file", func_reorient, "in_file")
Expand Down Expand Up @@ -1290,7 +1290,7 @@ def bold_mask_anatomical_refined(wf, cfg, strat_pool, pipe_num, opt=None):
mem_x=(0.0115, "in_file", "t"),
)

func_reorient.inputs.orientation = "RPI"
func_reorient.inputs.orientation = cfg.pipeline_setup["desired_orientation"]
func_reorient.inputs.outputtype = "NIFTI_GZ"

wf.connect(func_deoblique, "out_file", func_reorient, "in_file")
Expand Down
1 change: 1 addition & 0 deletions CPAC/longitudinal_pipeline/longitudinal_workflow.py
Original file line number Diff line number Diff line change
Expand Up @@ -1204,6 +1204,7 @@ def func_longitudinal_template_wf(subject_id, strat_list, config):
resampled_template.inputs.template = template
resampled_template.inputs.template_name = template_name
resampled_template.inputs.tag = tag
resampled_template.inputs.orientation = config["desired_orientation"]

strat_init.update_resource_pool(
{template_name: (resampled_template, "resampled_template")}
Expand Down
75 changes: 48 additions & 27 deletions CPAC/pipeline/engine.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
import warnings

from nipype import config, logging
from nipype.interfaces import afni
from nipype.interfaces.utility import Rename

from CPAC.image_utils.spatial_smoothing import spatial_smoothing
Expand All @@ -35,7 +36,11 @@
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,
)
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 @@ -2403,7 +2408,7 @@ def strip_template(data_label, dir_path, filename):
return data_label, json


def ingress_pipeconfig_paths(cfg, rpool, unique_id, creds_path=None):
def ingress_pipeconfig_paths(wf, cfg, rpool, unique_id, creds_path=None):
birajstha marked this conversation as resolved.
Show resolved Hide resolved
# ingress config file paths
# TODO: may want to change the resource keys for each to include one level up in the YAML as well

Expand All @@ -2412,6 +2417,7 @@ def ingress_pipeconfig_paths(cfg, rpool, unique_id, creds_path=None):

template_csv = p.resource_filename("CPAC", "resources/cpac_templates.csv")
template_df = pd.read_csv(template_csv, keep_default_na=False)
desired_orientation = cfg.pipeline_setup["desired_orientation"]

for row in template_df.itertuples():
key = row.Key
Expand Down Expand Up @@ -2468,32 +2474,29 @@ def ingress_pipeconfig_paths(cfg, rpool, unique_id, creds_path=None):

resampled_template = pe.Node(
Function(
input_names=["resolution", "template", "template_name", "tag"],
input_names=[
"orientation",
"resolution",
"template",
"template_name",
"tag",
],
output_names=["resampled_template"],
function=resolve_resolution,
as_module=True,
),
name="resampled_" + key,
)

resampled_template.inputs.orientation = desired_orientation
resampled_template.inputs.resolution = resolution
resampled_template.inputs.template = val
resampled_template.inputs.template_name = key
resampled_template.inputs.tag = tag

# the set_data below is set up a little differently, because we are
# injecting and also over-writing already-existing entries
# other alternative would have been to ingress into the
# resampled_template node from the already existing entries, but we
# didn't do that here
rpool.set_data(
key,
resampled_template,
"resampled_template",
json_info,
"",
"template_resample",
) # pipe_idx (after the blank json {}) should be the previous strat that you want deleted! because you're not connecting this the regular way, you have to do it manually
node = resampled_template
output = "resampled_template"
node_name = "template_resample"

elif val:
config_ingress = create_general_datasource(f"gather_{key}")
Expand All @@ -2503,14 +2506,33 @@ def ingress_pipeconfig_paths(cfg, rpool, unique_id, creds_path=None):
creds_path=creds_path,
dl_dir=cfg.pipeline_setup["working_directory"]["path"],
)
rpool.set_data(
key,
config_ingress,
"outputspec.data",
json_info,
"",
f"{key}_config_ingress",
)
node = config_ingress
output = "outputspec.data"
node_name = f"{key}_config_ingress"

if val.endswith(".nii" or ".nii.gz"):
check_reorient = pe.Node(
interface=afni.Resample(),
name=f"reorient_{key}",
)

check_reorient.inputs.orientation = desired_orientation
check_reorient.inputs.outputtype = "NIFTI_GZ"

wf.connect(node, output, check_reorient, "in_file")
node = check_reorient
output = "out_file"
node_name = f"{key}_reorient"
birajstha marked this conversation as resolved.
Show resolved Hide resolved

rpool.set_data(
key,
node,
output,
json_info,
"",
node_name,
)

# templates, resampling from config
"""
template_keys = [
Expand Down Expand Up @@ -2596,8 +2618,7 @@ def _set_nested(attr, keys):
)
cfg.set_nested(cfg, key, node)
"""

return rpool
return wf, rpool


def initiate_rpool(wf, cfg, data_paths=None, part_id=None):
Expand Down Expand Up @@ -2668,7 +2689,7 @@ def initiate_rpool(wf, cfg, data_paths=None, part_id=None):
)

# grab any file paths from the pipeline config YAML
rpool = ingress_pipeconfig_paths(cfg, rpool, unique_id, creds_path)
wf, rpool = ingress_pipeconfig_paths(wf, cfg, rpool, unique_id, creds_path)

# output files with 4 different scans

Expand Down
3 changes: 3 additions & 0 deletions CPAC/pipeline/schema.py
Original file line number Diff line number Diff line change
Expand Up @@ -423,6 +423,9 @@ def sanitize(filename):
"skip env check": Maybe(bool), # flag for skipping an environment check
"pipeline_setup": {
"pipeline_name": All(str, Length(min=1), sanitize),
"desired_orientation": In(
{"RPI", "LPI", "RAI", "LAI", "RAS", "LAS", "RPS", "LPS"}
),
"output_directory": {
"path": str,
"source_outputs_dir": Maybe(str),
Expand Down
2 changes: 1 addition & 1 deletion CPAC/pipeline/test/test_engine.py
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ def test_ingress_pipeconfig_data(pipe_config, bids_dir, test_dir):

rpool = ResourcePool(name=unique_id, cfg=cfg)

rpool = ingress_pipeconfig_paths(cfg, rpool, sub_data_dct, unique_id)
wf, rpool = ingress_pipeconfig_paths(wf, cfg, rpool, sub_data_dct, unique_id)

rpool.gather_pipes(wf, cfg, all=True)

Expand Down
2 changes: 1 addition & 1 deletion CPAC/registration/registration.py
Original file line number Diff line number Diff line change
Expand Up @@ -1736,7 +1736,7 @@ def ANTs_registration_connector(
"ANTs"
]["use_lesion_mask"]:
# Create lesion preproc node to apply afni Refit and Resample
lesion_preproc = create_lesion_preproc(wf_name=f"lesion_preproc{symm}")
lesion_preproc = create_lesion_preproc(cfg, wf_name=f"lesion_preproc{symm}")
wf.connect(inputNode, "lesion_mask", lesion_preproc, "inputspec.lesion")
wf.connect(
lesion_preproc,
Expand Down
1 change: 1 addition & 0 deletions CPAC/registration/tests/mocks.py
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,7 @@ def configuration_strategy_mock(method="FSL"):
resampled_template.inputs.template = template
resampled_template.inputs.template_name = template_name
resampled_template.inputs.tag = tag
resampled_template.inputs.orientation = "RPI"

strat.update_resource_pool(
{template_name: (resampled_template, "resampled_template")}
Expand Down
2 changes: 1 addition & 1 deletion CPAC/registration/tests/test_registration.py
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ def test_registration_lesion():

anat_preproc.inputs.inputspec.anat = anat_file

lesion_preproc = create_lesion_preproc(wf_name="lesion_preproc")
lesion_preproc = create_lesion_preproc(cfg, wf_name="lesion_preproc")

lesion_preproc.inputs.inputspec.lesion = lesion_file

Expand Down
4 changes: 4 additions & 0 deletions CPAC/resources/configs/pipeline_config_blank.yml
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,10 @@ pipeline_setup:
# Name for this pipeline configuration - useful for identification.
# This string will be sanitized and used in filepaths
pipeline_name: cpac-blank-template

# Desired orientation for the output data. "RPI", "LPI", "RAI", "LAI", "RAS", "LAS", "RPS", "LPS"
desired_orientation: RPI

output_directory:

# Quality control outputs
Expand Down
9 changes: 5 additions & 4 deletions CPAC/resources/tests/test_templates.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
import os

import pytest
import nipype.pipeline.engine as pe

from CPAC.pipeline import ALL_PIPELINE_CONFIGS
from CPAC.pipeline.engine import ingress_pipeconfig_paths, ResourcePool
Expand All @@ -29,11 +30,11 @@
@pytest.mark.parametrize("pipeline", ALL_PIPELINE_CONFIGS)
def test_packaged_path_exists(pipeline):
"""
Check that all local templates are included in image at at
least one resolution.
Check that all local templates are included in image at atleast one resolution.
"""
rpool = ingress_pipeconfig_paths(
Preconfiguration(pipeline), ResourcePool(), "pytest"
wf = pe.Workflow(name="test")
wf, rpool = ingress_pipeconfig_paths(
wf, Preconfiguration(pipeline), ResourcePool(), "pytest"
)
for resource in rpool.rpool.values():
node = next(iter(resource.values())).get("data")[0]
Expand Down
3 changes: 2 additions & 1 deletion CPAC/utils/datasource.py
Original file line number Diff line number Diff line change
Expand Up @@ -1156,7 +1156,7 @@ def res_string_to_tuple(resolution):
return (float(resolution.replace("mm", "")),) * 3


def resolve_resolution(resolution, template, template_name, tag=None):
def resolve_resolution(orientation, resolution, template, template_name, tag=None):
birajstha marked this conversation as resolved.
Show resolved Hide resolved
"""Resample a template to a given resolution."""
from nipype.interfaces import afni

Expand Down Expand Up @@ -1203,6 +1203,7 @@ def resolve_resolution(resolution, template, template_name, tag=None):
resample.inputs.resample_mode = "Cu"
resample.inputs.in_file = local_path
resample.base_dir = "."
resample.inputs.orientation = orientation

resampled_template = resample.run()
local_path = resampled_template.outputs.out_file
Expand Down
1 change: 1 addition & 0 deletions CPAC/utils/test_mocks.py
Original file line number Diff line number Diff line change
Expand Up @@ -235,6 +235,7 @@ def configuration_strategy_mock(method="FSL"):
resampled_template.inputs.template = template
resampled_template.inputs.template_name = template_name
resampled_template.inputs.tag = tag
resampled_template.inputs.orientation = "RPI"

strat.update_resource_pool(
{template_name: (resampled_template, "resampled_template")}
Expand Down
Loading