Allow relative paths in run_fastsurfer.sh

[ClePol]

This PR resolves relative paths in run_fastsurfer.sh before running the pipeline.

[Copilot]

Pull Request Overview

This PR adds functionality to resolve relative paths to absolute paths in the run_fastsurfer.sh script before executing the pipeline. This ensures that path variables used throughout the script are consistently absolute, which can prevent issues when the working directory changes during execution.

• Adds a helper function convert_to_absolute_path() to resolve paths using realpath
• Applies path resolution to 14 key path variables including input files, output files, and configuration paths
• Includes error handling with warning messages for failed path conversions

---

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

[Copilot]

[nitpick] The condition [ \"${var_value#*/}\" != \"$var_value\" ] is a complex way to check if a path contains directory separators. Consider using a more readable approach like [[ \"$var_value\" == */* ]] for better maintainability.

Suggested change

	if [ -e "$var_value" ] || [ "${var_value#*/}" != "$var_value" ]; then
	if [ -e "$var_value" ] || [[ "$var_value" == */* ]]; then

[dkuegler]

Relative paths have a meaning for almost all files listed in the PR. It means relative path to the subject_dir. So the only "legitimately" improperly supported paths would be relative paths for --t1 and --t2. Additionally, we could consider subjects_dir to be allowed to be relative.

I have to look at whether there is some logic with T1 being absolute paths, I think there was something...

[dkuegler]

I am not certain we "fully" want this behavior. We should discuss this, in the past, it was quite clear in the documentation, that t1 and t2 should be absolute paths. For all other files, relative should mean relative to the subject_dir. If this does for some reason not work, that would be a bug.

[dkuegler]

All these should not be made absolute

Suggested change

	convert_to_absolute_path "seg_log"
	convert_to_absolute_path "conformed_name"
	convert_to_absolute_path "norm_name"
	convert_to_absolute_path "norm_name_t2"
	convert_to_absolute_path "asegdkt_segfile"
	convert_to_absolute_path "mask_name"
	convert_to_absolute_path "merged_segfile"
	convert_to_absolute_path "cereb_segfile"
	convert_to_absolute_path "cereb_statsfile"
	convert_to_absolute_path "hypo_segfile"
	convert_to_absolute_path "hypo_statsfile"
	convert_to_absolute_path "asegdkt_vinn_statsfile"
	convert_to_absolute_path "aseg_vinn_statsfile"
	convert_to_absolute_path "aseg_segfile"

[dkuegler]

I would prefer the way this works to be like this

Suggested change

	convert_to_absolute_path "sd"
	sd="$(convert_to_absolute_path "$sd")"

The other version may work, but function that affect variables as a side effect are really ugly and should always be avoided.

[dkuegler]

how you can achieve the var=$(func "$val")" syntax is by echo-ing the return value and sending warnings and error messages to stderr.

Suggested change

	local var_name="$1"
	local var_value
	eval "var_value=\$$var_name"
	if [ -n "$var_value" ]; then
	if [ -e "$var_value" ] || [ "${var_value#*/}" != "$var_value" ]; then
	# Path exists or contains directory separators - convert to absolute
	local abs_path
	abs_path=$(realpath "$var_value" 2>/dev/null)
	if [ $? -eq 0 ] && [ -n "$abs_path" ]; then
	eval "$var_name=\"$abs_path\""
	else
	echo "WARNING: Failed to convert $var_name to absolute path: $var_value" | tee -a "${tmpLF:-/dev/null}"
	# usage: convert_to_absolute_path <path> [<logfile>]
	local var_value="$1"
	local logging=("cat")
	if [[ "$#" > 1]] ; then logging=(tee -a "$2") ; fi
	if [ -n "$var_value" ]; then
	if [ -e "$var_value" ] || [ "${var_value#*/}" != "$var_value" ]; then
	# Path exists or contains directory separators - convert to absolute
	local abs_path
	abs_path=$(realpath "$var_value" 2>/dev/null)
	if [ $? -eq 0 ] && [ -n "$abs_path" ]; then
	echo "$abs_path"
	else
	echo "WARNING: Failed to convert $var_name to absolute path: $var_value" | "${logging[@]}" &>2

Overall, this function should probably be moved to functions.sh.

Also, logging can be done more elegantly like indicated.

[dkuegler]

Technically, --t1 and --t2 are documented as follows:

  --t1  <T1_input>        T1 full head input (not bias corrected). Requires an
                            ABSOLUTE Path!
  --t2 <T2_input>         *Optional* T2 full head input (does not have to be bias
                            corrected, a mandatory biasfield correction step is
                            performed). Requires an ABSOLUTE Path!

Also forcing t1 to be an absolute path will probably break longitudinal segmentation, see

  if [[ -n "$t1" ]] && [[ "$t1" != "from-base" ]]; then
    echo "WARNING: --t1 was passed but will be overwritten with T1 from base template." | tee -a "$tmpLF"
  fi

Also, the t1 input check checks if the t1 file exists.

if [[ "$run_seg_pipeline" == "1" ]] && { [[ -z "$t1" ]] || [[ ! -f "$t1" ]]; }
then
  echo "ERROR: T1 image ($t1) could not be found. You must supply an existing T1 input"
  echo "  (full head) via --t1 <absolute path and name> for generating the segmentation."
  echo "NOTE: If running in a container, make sure symlinks are valid!"
  exit 1
fi

But this can probably be fixed by moving the conversion block to "after" the input checking and sanitization (search for comment #### START ####)

[dkuegler]

After more reviewing on #727, I am now convinced, that we do not want the functionality presented here. The logic is as follows

RULE:
All paths that are relative, are intended to be relative to the subject_dir.

As is, this makes relative paths relative to the working directory. That is not the current standard and not intended.
So in order to reduce confusion, we should not mix the two. We also have not done that in the past.

Course of Action:
Improve

1. the documentation better communicate how relative paths behave in FastSurfer and
2. the code to "enforce" absolute paths where necessary.

[ClePol]

Closing this, since it seems to require a different solution.