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.

Sign up for GitHub

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

Add nichart mode #1320

Merged
merged 7 commits into from
Nov 17, 2024
Merged

Add nichart mode #1320

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
9cbed28
Add nichart mode.
tsalo Nov 5, 2024
7945881
Update parser.py
tsalo Nov 5, 2024
595abba
Update parser_utils.py
tsalo Nov 5, 2024
a3e0db6
Fix tests.
tsalo Nov 5, 2024
ea68e07
Update test_cli.py
tsalo Nov 5, 2024
773fee8
Merge branch 'main' into nicharts-mode
tsalo Nov 13, 2024
cfa1385
Merge branch 'main' into nicharts-mode
tsalo Nov 13, 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
55 changes: 48 additions & 7 deletions xcp_d/cli/parser.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ def _build_parser():
'--mode',
dest='mode',
action='store',
choices=['abcd', 'hbcd', 'linc', 'none'],
choices=['abcd', 'hbcd', 'linc', 'nichart', 'none'],
required=True,
help=(
'The mode of operation for XCP-D. '
Expand Down Expand Up @@ -285,9 +285,10 @@ def _build_parser():
g_param.add_argument(
'--smoothing',
dest='smoothing',
default=6,
default='auto',
action='store',
type=float,
type=parser_utils._float_or_auto,
metavar='{{auto,FLOAT}}',
help=(
'FWHM, in millimeters, of the Gaussian smoothing kernel to apply to the denoised BOLD '
'data. '
Expand Down Expand Up @@ -531,13 +532,13 @@ def _build_parser():
'--min_coverage',
dest='min_coverage',
required=False,
default=0.5,
default='auto',
type=parser_utils._restricted_float,
metavar='{auto,FLOAT}',
help=(
'Coverage threshold to apply to parcels in each atlas. '
'Any parcels with lower coverage than the threshold will be replaced with NaNs. '
'Must be a value between zero and one, indicating proportion of the parcel. '
'Default is 0.5.'
'Must be a value between zero and one, indicating proportion of the parcel.'
),
)

Expand Down Expand Up @@ -939,7 +940,13 @@ def _validate_parameters(opts, build_log, parser):
assert opts.despike in (True, False, 'auto')
assert opts.file_format in ('nifti', 'cifti', 'auto')
assert opts.linc_qc in (True, False, 'auto')
assert opts.mode in ('abcd', 'hbcd', 'linc', 'none'), f"Unsupported mode '{opts.mode}'."
assert opts.mode in (
'abcd',
'hbcd',
'linc',
'nichart',
'none',
), f'Unsupported mode "{opts.mode}".'
assert opts.output_type in ('censored', 'interpolated', 'auto')
assert opts.process_surfaces in (True, False, 'auto')

Expand Down Expand Up @@ -968,12 +975,14 @@ def _validate_parameters(opts, build_log, parser):
opts.file_format = 'cifti' if (opts.file_format == 'auto') else opts.file_format
opts.input_type = 'fmriprep' if opts.input_type == 'auto' else opts.input_type
opts.linc_qc = True if (opts.linc_qc == 'auto') else opts.linc_qc
opts.min_coverage = 0.5 if opts.min_coverage == 'auto' else opts.min_coverage
if opts.motion_filter_type is None:
error_messages.append(f"'--motion-filter-type' is required for '{opts.mode}' mode.")
opts.output_correlations = True if 'all' in opts.dcan_correlation_lengths else False
if opts.output_type == 'censored':
error_messages.append(f"'--output-type' cannot be 'censored' for '{opts.mode}' mode.")
opts.output_type = 'interpolated'
opts.smoothing = 6 if opts.smoothing == 'auto' else opts.smoothing
opts.confounds_config = '36P' if opts.confounds_config == 'auto' else opts.confounds_config
opts.process_surfaces = (
True if (opts.process_surfaces == 'auto') else opts.process_surfaces
Expand All @@ -994,12 +1003,14 @@ def _validate_parameters(opts, build_log, parser):
opts.file_format = 'cifti' if (opts.file_format == 'auto') else opts.file_format
opts.input_type = 'nibabies' if opts.input_type == 'auto' else opts.input_type
opts.linc_qc = True if (opts.linc_qc == 'auto') else opts.linc_qc
opts.min_coverage = 0.5 if opts.min_coverage == 'auto' else opts.min_coverage
if opts.motion_filter_type is None:
error_messages.append(f"'--motion-filter-type' is required for '{opts.mode}' mode.")
opts.output_correlations = True if 'all' in opts.dcan_correlation_lengths else False
if opts.output_type == 'censored':
error_messages.append(f"'--output-type' cannot be 'censored' for '{opts.mode}' mode.")
opts.output_type = 'interpolated'
opts.smoothing = 6 if opts.smoothing == 'auto' else opts.smoothing
opts.confounds_config = '36P' if opts.confounds_config == 'auto' else opts.confounds_config
opts.process_surfaces = (
True if (opts.process_surfaces == 'auto') else opts.process_surfaces
Expand All @@ -1017,16 +1028,40 @@ def _validate_parameters(opts, build_log, parser):
opts.file_format = 'cifti' if (opts.file_format == 'auto') else opts.file_format
opts.input_type = 'fmriprep' if opts.input_type == 'auto' else opts.input_type
opts.linc_qc = True if (opts.linc_qc == 'auto') else opts.linc_qc
opts.min_coverage = 0.5 if opts.min_coverage == 'auto' else opts.min_coverage
opts.output_correlations = True
if opts.output_type == 'interpolated':
error_messages.append(
f"'--output-type' cannot be 'interpolated' for '{opts.mode}' mode."
)
opts.output_type = 'censored'
opts.smoothing = 6 if opts.smoothing == 'auto' else opts.smoothing
opts.confounds_config = '36P' if opts.confounds_config == 'auto' else opts.confounds_config
opts.process_surfaces = False if opts.process_surfaces == 'auto' else opts.process_surfaces
if opts.dcan_correlation_lengths is not None:
error_messages.append(f"'--create-matrices' is not supported for '{opts.mode}' mode.")
elif opts.mode == 'nichart':
opts.abcc_qc = False if (opts.abcc_qc == 'auto') else opts.abcc_qc
opts.combine_runs = False if opts.combine_runs == 'auto' else opts.combine_runs
opts.confounds_config = (
'36P' if (opts.confounds_config == 'auto') else opts.confounds_config
)
opts.dcan_correlation_lengths = (
'all' if opts.dcan_correlation_lengths is None else opts.dcan_correlation_lengths
)
opts.despike = True if (opts.despike == 'auto') else opts.despike
opts.fd_thresh = 0 if (opts.fd_thresh == 'auto') else opts.fd_thresh
opts.file_format = 'nifti' if (opts.file_format == 'auto') else opts.file_format
opts.input_type = 'fmriprep' if opts.input_type == 'auto' else opts.input_type
opts.linc_qc = True if (opts.linc_qc == 'auto') else opts.linc_qc
opts.min_coverage = 0.4 if opts.min_coverage == 'auto' else opts.min_coverage
opts.output_correlations = True if 'all' in opts.dcan_correlation_lengths else False
opts.output_type = 'censored' if opts.output_type == 'auto' else opts.output_type
opts.smoothing = 0 if opts.smoothing == 'auto' else opts.smoothing
opts.confounds_config = '36P' if opts.confounds_config == 'auto' else opts.confounds_config
opts.process_surfaces = False if opts.process_surfaces == 'auto' else opts.process_surfaces
# Remove "all" from the list of correlation lengths
opts.dcan_correlation_lengths = [c for c in opts.dcan_correlation_lengths if c != 'all']
elif opts.mode == 'none':
if opts.abcc_qc == 'auto':
error_messages.append("'--abcc-qc' (y or n) is required for 'none' mode.")
Expand Down Expand Up @@ -1057,6 +1092,9 @@ def _validate_parameters(opts, build_log, parser):
if opts.linc_qc == 'auto':
error_messages.append("'--linc-qc' (y or n) is required for 'none' mode.")

if opts.min_coverage == 'auto':
error_messages.append("'--min-coverage' is required for 'none' mode.")

if opts.motion_filter_type is None:
error_messages.append("'--motion-filter-type' is required for 'none' mode.")

Expand All @@ -1070,6 +1108,9 @@ def _validate_parameters(opts, build_log, parser):
"'--warp-surfaces-native2std' (y or n) is required for 'none' mode."
)

if opts.smoothing == 'auto':
error_messages.append("'--smoothing' is required for 'none' mode.")

# Remove "all" from the list of correlation lengths
opts.dcan_correlation_lengths = [c for c in opts.dcan_correlation_lengths if c != 'all']

Expand Down
3 changes: 3 additions & 0 deletions xcp_d/cli/parser_utils.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,9 @@ def _float_or_auto_or_none(string, is_parser=True):

def _restricted_float(x):
"""From https://stackoverflow.com/a/12117065/2589328."""
if x == 'auto':
return x

try:
x = float(x)
except ValueError as exc:
Expand Down
2 changes: 2 additions & 0 deletions xcp_d/tests/test_cli.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,9 @@ def test_ds001419_nifti(data_dir, output_dir, working_dir):
'--motion-filter-type=lp',
'--band-stop-min=6',
'--skip-parcellation',
'--min-coverage=0.4',
'--min-time=100',
'--smoothing=6',
'--combine-runs',
'--output-type=censored',
'--combine-runs=y',
Expand Down
29 changes: 28 additions & 1 deletion xcp_d/tests/test_cli_run.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,10 +45,12 @@ def base_opts():
'motion_filter_order': None,
'process_surfaces': 'auto',
'atlases': ['Glasser'],
'min_coverage': 'auto',
'dcan_correlation_lengths': None,
'despike': 'auto',
'abcc_qc': 'auto',
'linc_qc': 'auto',
'smoothing': 'auto',
'combine_runs': 'auto',
'output_type': 'auto',
'fs_license_file': None,
Expand Down Expand Up @@ -276,6 +278,8 @@ def test_validate_parameters_linc_mode(base_opts, base_parser, capsys):
assert opts.abcc_qc is False
assert opts.linc_qc is True
assert opts.file_format == 'cifti'
assert opts.min_coverage == 0.5
assert opts.smoothing == 6.0

# --create-matrices is not supported
opts.dcan_correlation_lengths = [300]
Expand Down Expand Up @@ -304,8 +308,10 @@ def test_validate_parameters_abcd_mode(base_opts, base_parser, capsys):
assert opts.file_format == 'cifti'
assert opts.input_type == 'fmriprep'
assert opts.linc_qc is True
assert opts.min_coverage == 0.5
assert opts.output_correlations is False
assert opts.process_surfaces is True
assert opts.smoothing == 6.0

opts.dcan_correlation_lengths = ['300', 'all']
opts = parser._validate_parameters(deepcopy(opts), build_log, parser=base_parser)
Expand Down Expand Up @@ -339,8 +345,10 @@ def test_validate_parameters_hbcd_mode(base_opts, base_parser, capsys):
assert opts.file_format == 'cifti'
assert opts.input_type == 'nibabies'
assert opts.linc_qc is True
assert opts.min_coverage == 0.5
assert opts.output_correlations is False
assert opts.process_surfaces is True
assert opts.smoothing == 6.0

opts.dcan_correlation_lengths = ['300', 'all']
opts = parser._validate_parameters(deepcopy(opts), build_log, parser=base_parser)
Expand All @@ -356,6 +364,21 @@ def test_validate_parameters_hbcd_mode(base_opts, base_parser, capsys):
assert "'--motion-filter-type' is required for" in stderr


def test_validate_parameters_nichart_mode(base_opts, base_parser, capsys):
"""Test parser._validate_parameters with nichart mode."""
opts = deepcopy(base_opts)
opts.mode = 'nichart'

# linc mode doesn't use abcc_qc but does use linc_qc
opts = parser._validate_parameters(deepcopy(opts), build_log, parser=base_parser)

assert opts.abcc_qc is False
assert opts.linc_qc is True
assert opts.file_format == 'nifti'
assert opts.min_coverage == 0.4
assert opts.smoothing == 0


def test_validate_parameters_none_mode(base_opts, base_parser, capsys):
"""Test parser._validate_parameters with none mode."""
opts = deepcopy(base_opts)
Expand All @@ -372,9 +395,11 @@ def test_validate_parameters_none_mode(base_opts, base_parser, capsys):
assert "'--file-format' is required for 'none' mode." in stderr
assert "'--input-type' is required for 'none' mode." in stderr
assert "'--linc-qc' (y or n) is required for 'none' mode." in stderr
assert "'--min-coverage' is required for 'none' mode." in stderr
assert "'--motion-filter-type' is required for 'none' mode." in stderr
assert "'--nuisance-regressors' is required for 'none' mode." in stderr
assert "'--output-type' is required for 'none' mode." in stderr
assert "'--smoothing' is required for 'none' mode." in stderr
assert "'--warp-surfaces-native2std' (y or n) is required for 'none' mode." in stderr

opts.abcc_qc = False
Expand All @@ -385,10 +410,12 @@ def test_validate_parameters_none_mode(base_opts, base_parser, capsys):
opts.file_format = 'nifti'
opts.input_type = 'fmriprep'
opts.linc_qc = False
opts.min_coverage = 0.5
opts.motion_filter_type = 'none'
opts.output_type = 'censored'
opts.params = '36P'
opts.process_surfaces = False
opts.smoothing = 0
opts = parser._validate_parameters(deepcopy(opts), build_log, parser=base_parser)


Expand All @@ -397,7 +424,7 @@ def test_validate_parameters_other_mode(base_opts, base_parser, capsys):
opts = deepcopy(base_opts)
opts.mode = 'other'

with pytest.raises(AssertionError, match="Unsupported mode 'other'"):
with pytest.raises(AssertionError, match='Unsupported mode "other"'):
parser._validate_parameters(deepcopy(opts), build_log, parser=base_parser)


Expand Down