Make the code more generalizable to non-human template #53

NadiaBlostein · 2023-07-10T15:51:31Z

This PR is about executing steps 1.1 to 1.4 in (see README).

In order to test this PR:

clone the repository, and get the latest version of this branch
```
git checkout nb/preprocess_segment
git pull
```

get the data:

git clone [email protected]:datasets/philadelphia-pediatric
git checkout a99400038a98d074e79e69b955aec6d6fefe2abb
git-annex get sub-101
git-annex get sub-102

edit config file using these info:

{
    "path_data": "ABSOLUTE/PATH/TO/DATA/"
    "include-list": "sub-101 sub-102",
    "data_type": "anat",
    "contrast": "t1",
    "suffix_image": "_rec-composed_T1w",
    "first_disc": "1",
    "last_disc": "18"
}

Test this PR
- Read the README
- Test section 1.1 --> 1.4

Before testing this PR, this should be done: neuropoly/data-management#248

Fixes #25, #29, #31, #34, #37, #50, #51

-- Spinal Cord Toolbox (git-master-e740edf4c8408ffa44ef7ba23ad068c6d07e4b87) sct_run_batch --

…segment.sh

…e more compatible with sct_run_batch include-list

jcohenadad · 2023-07-10T18:08:11Z

preprocess_normalize.py

+        coord = im_discs.getNonZeroCoordinates(sorting = 'z', reverse_coord = True)
+        coord_physical = []
+        for c in coord:
+            if c.value <= last_disc or c.value in [48, 49, 50, 51, 52]:


could this be a problem in non-human templates?

jcohenadad · 2023-07-10T18:08:36Z

preprocess_normalize.py

-            centerline.save_centerline(fname_output = fname_centerline)
+            print(subject_name + ' SC segmentation does not exist. Extracting centerline from ' + fname_image)
+            im_seg = Image(fname_image).change_orientation('RPI')
+            param_centerline = ParamCenterline(algo_fitting = 'optic', smooth = smooth, degree = 5, minmax = minmax)


jcohenadad · 2023-07-10T18:09:44Z

preprocess_normalize.py


        list_centerline.append(centerline)
        tqdm_bar.update(1)
    tqdm_bar.close()
    os.chdir(current_path)
    return list_centerline

-# def compute_ICBM152_centerline(dataset_info): ###### FIX
+# def compute_ICBM152_centerline(dataset_info): # ??????


replace the ????? by an issue on GH that points to the line of code that you don't understand

jcohenadad

As per previous discussions, the configuration file should not include ALL parameters, but only parameters that enable to reproduce the experiments. Parameters that should not be included include: jobs (bc it depends on local hardware), path-output (bc it depends on prefered user organization),
path_data and path_output should be with "-" instead of "_". Please change everywhere appropriate
script: remove from config file
path_output --> should not be in the config file

…nd updating README according to how users should run 'sct_run_batch'

… ??????'

jcohenadad · 2023-07-10T20:21:09Z

Suggestions about PATH_OUT. If setting PATH_OUT to be PATH_DATA/derivatives/labels, the following will happen:

├── sub-XXX  <---- your dataset
│   └── anat
│       └──sub-XXX_T1w.nii.gz
...
...
└── derivatives
    └── labels
        ├── results  <---- empty
        ├── qc  <---- has stuff
        ├── log  <---- has stuff
        ├── processed_data  <---- empty
        ├── sub-XXX
        │   └── anat
        │       └──sub-XXX_T1w_labelYYY.nii.gz  <---- segmentation and/or disc label to use for template generation
        ...

In general, PATH_OUT would be set to a local directory (eg: scratch space on a cluster), and then, the useful data would be copied back into the dataset under derivatives/labels.

However, I could see the advantages of the proposed approach:

no need to copy from PATH_OUT to the input data folder (ie: less prone to human error)
more logging info associated with the processing.

Cons:

the dataset becomes cluttered with additional processing qc/logging, which is not the end the world...

preprocess_segment.sh

some TODOs remain

jcohenadad · 2023-07-12T15:50:03Z

preprocess_segment.sh

+FILE="${SUBJECT}${IMAGE_SUFFIX}.nii.gz"
+FILESEG="${SUBJECT}${IMAGE_SUFFIX}_label-SC_seg.nii.gz"


should be looking in the derivative folder of the data

jcohenadad · 2023-07-12T15:52:17Z

preprocess_segment.sh

+  echo "Not found. Proceeding with automatic segmentation."
+  # Segment spinal cord
+  sct_deepseg_sc -i ${FILE} -o ${FILESEG} -c ${CONTRAST} -qc ${PATH_QC} -qc-subject ${SUBJECT}
+  # TODO: MOVE THAT FILE UNDER derivatives/labels


@NadiaBlostein after thinking more about it, I think we should not move the outputs in the derivatives folder of the dataset automatically. The reason is that, if someone tries running the script, it will overwrite the data in the derivatives, which will be an annoyance.

Instead, we should do this move while doing the manual correction of the segmentations/labels. In fact, this is what is currently done by other projects. Also see #58

Lines 73 and 89 of preprocess_segment.sh check if the files exist to avoid overwriting anything. This also allows the straightening.cache, straight_ref.nii.gz, warp_curve2straight.nii.gz and warp_straight2curve.nii.gz for each subject to be saved separately.

However, if you prefer us to remain consistent with what everyone else does, I’ll take a look through the other projects to find a repository that I could use as a blueprint for this.

preprocess_segment.py is not used anymore, is it? I’m not sure I understand your comment.

Also: straightening.cache etc. have nothing to do with my comment because these files were not copied anyway. I think there is a misunderstanding that will
likely be better resolved in a meeting.

My apologies, I made a typo (was on phone); was referring to preprocess_segment.sh. Will correct comment now.

I understood that you may have at least wanted the subject-specific warp files saved. We'll chat next week.

My apologies, I made a typo (was on phone); was referring to preprocess_segment.sh. Will correct comment now.

👍

I understood that you may have at least wanted the subject-specific warp files saved. We'll chat next week.

No. What I meant in the meeting is that we want these subject-specific files in their own folders (as opposed to a flat directory where there could be file conflicts)-- I did not mean that these data should be ultimately stored in the git-annexed source data

jcohenadad · 2023-07-12T15:52:31Z

preprocess_segment.sh

+  mv "${SUBJECT}${IMAGE_SUFFIX}_label-SC_seg_labeled_discs.nii.gz" "${SUBJECT}${IMAGE_SUFFIX}_label-disc.nii.gz"
+  # TODO: MOVE THAT FILE UNDER derivatives/labels
+  mv "${SUBJECT}${IMAGE_SUFFIX}_label-SC_seg_labeled.nii.gz" "${SUBJECT}${IMAGE_SUFFIX}_label-disc_levels.nii.gz"


~~should be moved to derivatives~~

scrap that (see https://github.com/neuropoly/template/pull/53/files#r1261649236)

jcohenadad · 2023-07-12T16:02:35Z

preprocess_segment.sh

 # Copy source images
-rsync -avzh $PATH_DATA/$SUBJECT .
+rsync -avzh $PATH_DATA/$SUBJECT/$DATA_TYPE/* .


all subject files should not be in a flat directory, but instead each subject in its own directory-- to avoid file conflicts-- please do it as we do in: https://spine-generic.readthedocs.io/analysis-pipeline.html

Nadia Blostein added 10 commits July 6, 2023 11:03

Issue #51

1c5889d

replacing with in to be able to use

e1cad11

-- Spinal Cord Toolbox (git-master-e740edf4c8408ffa44ef7ba23ad068c6d07e4b87) sct_run_batch --

fixing segmentation code; still requires tweaking output directories

82227db

adding options to configuration_default.json and updating preprocess_…

1c86c87

…segment.sh

changing separation between subject names to ' ' instead of ', ' to b…

8eb8ba2

…e more compatible with sct_run_batch include-list

Updating README according to new changes

c012f63

updating README

ae13a98

Solving issues #34 and #32

5b0aa6b

Issue #50

210040c

cleaning up code

be4308e

jcohenadad reviewed Jul 10, 2023

View reviewed changes

Update preprocess_normalize.py

dff8126

jcohenadad requested changes Jul 10, 2023

View reviewed changes

Nadia Blostein and others added 3 commits July 10, 2023 15:24

removing unecessary key-value pairs from configuration_default.json a…

ac6df6e

…nd updating README according to how users should run 'sct_run_batch'

removing my obscure comments that nobody else understands, such as '#…

db6a2ff

… ??????'

Update README.md

210fc5f

Added documentation about expected file structure

49901c8

jcohenadad reviewed Jul 10, 2023

View reviewed changes

preprocess_segment.sh Outdated Show resolved Hide resolved

jcohenadad added 2 commits July 10, 2023 16:52

Update preprocess_segment.sh

3e6b676

some TODOs remain

Update README.md

25267b7

jcohenadad changed the title ~~Nb/preprocess segment~~ Make the code more generalizable to non-human template Jul 12, 2023

jcohenadad mentioned this pull request Jul 12, 2023

Add corrected labels neuropoly/data-management#248

Closed

jcohenadad reviewed Jul 12, 2023

View reviewed changes

final changes

0ca0cd4

NadiaBlostein merged commit 81e9d93 into master Jul 12, 2023

NadiaBlostein deleted the nb/preprocess_segment branch July 12, 2023 19:58

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Make the code more generalizable to non-human template #53

Make the code more generalizable to non-human template #53

NadiaBlostein commented Jul 10, 2023 •

edited

Loading

jcohenadad Jul 10, 2023

jcohenadad Jul 10, 2023

jcohenadad Jul 10, 2023

jcohenadad left a comment

jcohenadad commented Jul 10, 2023

jcohenadad Jul 12, 2023

jcohenadad Jul 12, 2023

jcohenadad Jul 12, 2023

NadiaBlostein Jul 12, 2023 •

edited

Loading

jcohenadad Jul 13, 2023

NadiaBlostein Jul 13, 2023

jcohenadad Jul 13, 2023

jcohenadad Jul 12, 2023 •

edited

Loading

jcohenadad Jul 12, 2023

		FILE="${SUBJECT}${IMAGE_SUFFIX}.nii.gz"
		FILESEG="${SUBJECT}${IMAGE_SUFFIX}_label-SC_seg.nii.gz"

Make the code more generalizable to non-human template #53

Make the code more generalizable to non-human template #53

Conversation

NadiaBlostein commented Jul 10, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jcohenadad left a comment

Choose a reason for hiding this comment

jcohenadad commented Jul 10, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

NadiaBlostein Jul 12, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jcohenadad Jul 12, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

NadiaBlostein commented Jul 10, 2023 •

edited

Loading

NadiaBlostein Jul 12, 2023 •

edited

Loading

jcohenadad Jul 12, 2023 •

edited

Loading