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

Group mode #34

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
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
23 changes: 12 additions & 11 deletions make.template
Original file line number Diff line number Diff line change
Expand Up @@ -22,15 +22,16 @@ usage()
echo -n "structural_pipeline for structural connectivity reconstruction.

Synopsis:
structural_pipeline -s SUBJECTDIR -m MCRROOT [OPTION]...
structural_pipeline [STEPS]... -s SUBJECTDIR -m MCRROOT [OPTION]...
structural_pipeline -s SUBJECTSELECTION -m MCRROOT [OPTION]...
structural_pipeline [STEPS]... -s SUBJECTSELECTION -m MCRROOT [OPTION]...

Description:
CATO structural connectivity reconstruction pipeline. Execution of
specific pipeline steps can be requested on the command line by the STEPS.

Required arguments:
-s, --subjectDir Directory with a subject's Freesurfer and DWI data
-s, --subjectSelection Directory with a subject's Freesurfer and MRI data
or file with list of subject directories.
-m, --mcrRoot Installation directory of MATLAB runtime

Optional arguments:
Expand All @@ -53,12 +54,12 @@ networkSteps=
while [ -n "$1" ]; do
shopt -s nocasematch
case "$1" in
-s | --subjectDir)
subjectDir=$2
-s | --subjectSelection)
subjectSelection=$2
shift 2
;;
--subjectDir=*)
subjectDir=${1#*=}
--subjectSelection=*)
subjectSelection=${1#*=}
shift
;;
-m | --mcrRoot)
Expand Down Expand Up @@ -118,9 +119,9 @@ main() {

parse_input "$@"

if [[ ! -d "$subjectDir" ]]
if [[ ! -e "$subjectSelection" ]]
then
echo "Subject directory argument missing / not a directory."
echo "Subject selection argument missing / does not exist."
exit 1
fi

Expand Down Expand Up @@ -158,9 +159,9 @@ main() {

# Add requested reconstruction steps if specified.
if [ ! -z "$networkSteps" ]; then
$target "$subjectDir" "${NameValueParams[@]}" reconstructionSteps ${networkSteps// /,}
$target "$subjectSelection" "${NameValueParams[@]}" reconstructionSteps ${networkSteps// /,}
else
$target "$subjectDir" "${NameValueParams[@]}"
$target "$subjectSelection" "${NameValueParams[@]}"
fi

# clean up and exit
Expand Down
60 changes: 53 additions & 7 deletions src/functional_pipeline/functional_pipeline.m
Original file line number Diff line number Diff line change
@@ -1,10 +1,13 @@
function configParams = functional_pipeline(subjectDir, varargin)
function configParams = functional_pipeline(subjectSelection, varargin)
% FUNCTIONAL_PIPELINE Reconstruct structural connectivity.
%
% functional_pipeline(SUBJECTDIR) reconstructs functional connectivity of
% a subjects T1 and rs-fMRI data in the SUBJECTDIR directory using default
% parameters.
%
% functional_pipeline(SUBJECTLISTFILE) takes a file in which each line is
% a subject directory, and calls functional_pipeline for each directory.
%
% functional_pipeline(SUBJECTDIR, 'Param1', VAL1, 'Param2', VAL2, ...)
% specifies additional parameter name-value pairs chosen from:
%
Expand Down Expand Up @@ -62,20 +65,63 @@


% PARSE INPUT
[subjectDir, varargin{:}] = convertStringsToChars(subjectDir, varargin{:});
[subjectSelection, varargin{:}] = convertStringsToChars(subjectSelection, varargin{:});

assert(ischar(subjectDir), ...
assert(ischar(subjectSelection), ...
Copy link
Member

Choose a reason for hiding this comment

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

I would update subjectDirNotText to subjectSelectionNotText (here and at the other locations), such that the internal error ID matches the variable.

'CATO:functional_pipeline:subjectDirNotText', ...
'subjectDir must be a row vector of characters or string scalar.');
assert(isdir(subjectDir), ...
assert(isdir(subjectSelection) | isfile(subjectSelection), ...
Copy link
Member

Choose a reason for hiding this comment

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

The function isfolder is recommended instead of isdir:
https://www.mathworks.com/help/matlab/ref/isdir.html

'CATO:functional_pipeline:subjectDirNotDir', ...
'subjectDir (%s) is not a directory.', subjectDir);
'subjectDir (%s) is not a directory.', subjectSelection);

[configFile, runType, configParamsCl] = parseVarargin(varargin{:});
% parseVarargin does also error handling.
%% Group mode

if isfile(subjectSelection)
subjectListFile = subjectSelection;
fprintf('\n--------strarting group run----------\n')

% read file
fi = fopen(subjectListFile);
Copy link
Member

Choose a reason for hiding this comment

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

Instead of using fopen and fgetl functions, we can maybe use the readcell function to read the subject list file. readcell handles trailing white spaces at the end of lines (which occurs often accidentally) and provides better error handling, which makes it a more reliable option.

Additionally, using readcell allows us to load all subject directories at once and let the pipeline check that they are valid before running the pipeline. This way, the user can get information about the number of valid subjects and catch errors before running the long pipeline.

Maybe, readcell has some drawbacks that I missed, in that case these issues could also be addressed with e.g. deblank.

Small note: The file handle is commonly referred to as fid instead of fi in CATO.


fprintf('Processing subjects in %s\n\n', subjectListFile);

% read line by line, keep track of processed/failed/unrecognized
subjectDir = fgetl(fi);
processedSubjects = 0;
failedSubjects = 0;
unrecognizedSubjects = 0;

while ischar(subjectDir)
if isdir(subjectDir)
try
configParams = functional_pipeline(subjectDir, varargin{:});
processedSubjects = processedSubjects + 1;
catch
failedSubjects = failedSubjects + 1;
end
else
unrecognizedSubjects = unrecognizedSubjects + 1;
warning('CATO:functional_pipeline:subjectDirNotDir', ...
'subjectDir (%s) is not a directory', subjectDir);
end
subjectDir = fgetl(fi);
end

fprintf('\n---------group run finished----------\n')
fprintf('Processed %d subjects, %d failed, %d unrecognized\n\n', ...
Copy link
Member

Choose a reason for hiding this comment

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

Nice that the code keeps track of the unsuccessful subjects! It might also be a good idea to provide a list of the subject names that were unsuccessful as this might help users to identify issues quickly. Maybe this list can be displayed alongside the statistics?

To keep this CATO usable with this feature, it might be helpful to truncate the list to a manageable size (e.g. 100 or 1,000) when the number of subjects is very large.

processedSubjects, failedSubjects, unrecognizedSubjects);

fclose(fi);
return
end

subjectDir = subjectSelection;

%% Setup

[configFile, runType, configParamsCl] = parseVarargin(varargin{:});
% parseVarargin does also error handling.

% Setup path (note this is done before log files or other stuff).
oldPath = pwd;

Expand Down
58 changes: 52 additions & 6 deletions src/structural_pipeline/structural_pipeline.m
Original file line number Diff line number Diff line change
@@ -1,10 +1,13 @@
function configParams = structural_pipeline(subjectDir, varargin)
function configParams = structural_pipeline(subjectSelection, varargin)
% STRUCTURAL_PIPELINE Reconstruct structural connectivity.
%
% structural_pipeline(SUBJECTDIR) reconstructs structural connectivity of
% a subjects T1 and DWI data in the SUBJECTDIR directory using default
% parameters.
%
% structural_pipeline(SUBJECTLISTFILE) takes a file in which each line is
% a subject directory, and calls structural_pipeline for each directory.
%
% structural_pipeline(SUBJECTDIR, 'Param1', VAL1, 'Param2', VAL2, ...)
% specifies additional parameter name-value pairs chosen from:
%
Expand Down Expand Up @@ -61,19 +64,62 @@
clear toolboxInstalled

% PARSE INPUT
[subjectDir, varargin{:}] = convertStringsToChars(subjectDir, varargin{:});
[subjectSelection, varargin{:}] = convertStringsToChars(subjectSelection, varargin{:});

assert(ischar(subjectDir), ...
assert(ischar(subjectSelection), ...
'CATO:structural_pipeline:subjectDirNotText', ...
'subjectDir must be a row vector of characters or string scalar.');
assert(isdir(subjectDir), ...
assert(isdir(subjectSelection) | isfile(subjectSelection), ...
'CATO:structural_pipeline:subjectDirNotDir', ...
'subjectDir (%s) is not a directory', subjectDir);
'subjectSelection (%s) is not a directory or file', subjectSelection);

[configFile, runType, configParamsCl] = parseVarargin(varargin{:});
%% Group mode

if isfile(subjectSelection)
subjectListFile = subjectSelection;
fprintf('\n--------strarting group run----------\n')

% read file
fi = fopen(subjectListFile);

fprintf('Processing subjects in %s\n\n', subjectListFile);

% read line by line, keep track of processed/failed/unrecognized
subjectDir = fgetl(fi);
processedSubjects = 0;
failedSubjects = 0;
unrecognizedSubjects = 0;

while ischar(subjectDir)
if isdir(subjectDir)
try
configParams = structural_pipeline(subjectDir, varargin{:});
processedSubjects = processedSubjects + 1;
catch
failedSubjects = failedSubjects + 1;
end
else
unrecognizedSubjects = unrecognizedSubjects + 1;
warning('CATO:structural_pipeline:subjectDirNotDir', ...
'subjectDir (%s) is not a directory', subjectDir);
end
subjectDir = fgetl(fi);
end

fprintf('\n---------group run finished----------\n')
fprintf('Processed %d subjects, %d failed, %d unrecognized\n\n', ...
processedSubjects, failedSubjects, unrecognizedSubjects);

fclose(fi);
return
end

subjectDir = subjectSelection;

%% Setup

[configFile, runType, configParamsCl] = parseVarargin(varargin{:});

% Setup path (note this is done before log files or other stuff).
oldPath = pwd;

Expand Down