Make sct_get_centerline robust to intensities with range [0, 1] #1746

charleygros · 2018-05-17T14:28:43Z

Requirements

Is there a issue open describing the motivation for this pull request ?
Output cord segmentation is empty on image with reasonable contrast #1740

Description of the Change

If all intensities of the image are < 1, then this line:
https://github.com/neuropoly/spinalcordtoolbox/blob/master/spinalcordtoolbox/centerline/optic.py#L95
leads to a binarization of the input image --> fail of OptiC

Solution: make an intensity scaling prior to the int16 conversion.
Changes tested on ~ 50 images --> seems to do not affect OptiC.

Steps and Constraints

To test the pull request, please reinstall the toolbox because the changes are made in spinalcordtoolbox/optic

…y < 1

stephaniealley

The issue is resolved for me. I tested it on all 3 of the contrasts for which it failed in this manner, and the segmentations are now successfully generated.

jcohenadad

Wouldn't it be better to fix the issue inside sct_image instead?

jcohenadad · 2018-05-17T23:44:26Z

spinalcordtoolbox/centerline/optic.py

+def apply_intensity_normalization(fname_in, fname_out):
+    img = Image(fname_in)
+    img_normalized = img.copy()
+    p2, p98 = np.percentile(img.data, (2, 98))


Isn't int16 range -32,768 to 32,767?

I am sorry, I am not sure to understand: does the below comment helps? Otherwise, May I ask you to re-explain to me? Thank you

Wouldn't it be better to fix the issue inside sct_image instead?

Maybe! But I am not sure to see how, Could you please help me? Merci

sorry @charleygros i meant line 71 (below), where it says: out_range=(0, 255) (equivalent 8bit unsigned), whereas later in the code the data is converted to int16, hence we loose precision. Or am i missing something?

maybe skimage.img_as_uint could be useful here?

It seems to be a better solution, merci!

Just one question: The documentation says:

Negative input values will be clipped.

--> Could we assume that images will always have positive values?

i suggest we do, but we should check for it. If there are neg values, a "warning" should be sent.

charleygros · 2018-05-18T00:43:43Z

The issue here is:

cd /Volumes/sct_testing/issues/20180517_issue1740
sct_image -i 9611-0_0-images_017_jvgre10TE2mm1001_trace15Cav.nii.gz -type int16 -o 9611-0_0-images_017_jvgre10TE2mm1001_trace15Cav_int16.nii.gz

Leads to:

Concerning the -32,768 to 32,767:

cd /Volumes/sct_testing/issues/20180517_issue1740
python
>>> from msct_image import Image
>>> import numpy as np
>>> ii = Image('9611-0_0-images_017_jvgre10TE2mm1001_trace15Cav_-31_689_31_115999.nii.gz')
>>> np.min(ii.data), np.max(ii.data)
(-31.698, 31.115999)
>>> del ii
>>> quit()
sct_get_centerline -i 9611-0_0-images_017_jvgre10TE2mm1001_trace15Cav_-31_689_31_115999.nii.gz -c t2s

Leads to:

jcohenadad · 2018-05-18T00:50:17Z

Wouldn't it be better to fix the issue inside sct_image instead?

@charleygros I will look into it and provide suggestions

UPDATE 2018-05-18 10.20: There is already code inside msct_image.changeType() which is supposed to deal with that issue. Obviously this code is not working. So we should just fix that part, without the need to add a rescaling in sct_deepseg_sc: simply calling msct_image.changeType() should work.

charleygros · 2018-05-18T01:22:29Z

Image with intensities between -31.689 and 31.115999:

sct_get_centerline -i 9611-0_0-images_017_jvgre10TE2mm1001_trace15Cav_-31_689_31_115999.nii.gz -c t2s

leads to:

--
Spinal Cord Toolbox (cg_i1740/f2ba0738910525997abb8f3f4fca1c5cdd981335)
Running /Users/chgroc/spinalcordtoolbox/scripts/sct_get_centerline.py -i 9611-0_0-images_017_jvgre10TE2mm1001_trace15Cav_-31_689_31_115999.nii.gz -c t2s
Configuring sentry report
sentry is set!
Detecting the spinal cord using OptiC
cp /Volumes/sct_testing/issues/20180517_issue1740/9611-0_0-images_017_jvgre10TE2mm1001_trace15Cav_-31_689_31_115999.nii.gz /var/folders/0_/g93k0_9d53ggwtb6hxgmw10c19x4f8/T/sct-180517211726-1XTrCK
Traceback (most recent call last):
  File "/Users/chgroc/spinalcordtoolbox/scripts/sct_get_centerline.py", line 266, in <module>
    run_main()
  File "/Users/chgroc/spinalcordtoolbox/scripts/sct_get_centerline.py", line 261, in run_main
    verbose=verbose)
  File "/Users/chgroc/spinalcordtoolbox/spinalcordtoolbox/centerline/optic.py", line 108, in detect_centerline
    scale_intensities_uint16(image_fname, image_int_filename)
  File "/Users/chgroc/spinalcordtoolbox/spinalcordtoolbox/centerline/optic.py", line 70, in scale_intensities_uint16
    img_scaled.data = img_as_uint(img.data)
  File "/Users/chgroc/spinalcordtoolbox/python/lib/python2.7/site-packages/skimage/util/dtype.py", line 325, in img_as_uint
    return convert(image, np.uint16, force_copy)
  File "/Users/chgroc/spinalcordtoolbox/python/lib/python2.7/site-packages/skimage/util/dtype.py", line 205, in convert
    raise ValueError("Images of type float must be between -1 and 1.")
ValueError: Images of type float must be between -1 and 1.
Sentry is attempting to send 1 pending error messages
Waiting up to 10 seconds
Press Ctrl-C to quit
[1]+  Done                    fslview 9611-0_0-images_017_jvgre10TE2mm1001_trace15Cav.nii.gz -l Greyscale ./9611-0_0-images_017_jvgre10TE2mm1001_trace15Cav_centerline_optic.nii.gz -l Red -t 1

jcohenadad · 2018-05-18T01:35:26Z

in response to this comment: can't we do a rescaling, by adjusting the in_min and in_max to the out_min and out_max, as done here? That would solve issue with neg values.

… the "if" condition

charleygros · 2018-05-18T16:08:43Z

Travis showed error of sct_testing -f sct_propseg.
Reason:
https://github.com/neuropoly/spinalcordtoolbox/blob/master/scripts/sct_propseg.py#L530
The output of PropSeg is converted to int8 --> a scaling of [0, 1] leads to [-127, 128] --> Dice = 0, because of this commit:
f26a735

I would suggest to:

keep the former implementation of msct_image.changeType since removing the "if condition" will certainly break other functions
do the uint16-scaling in sct_get_centerline then change the type

…ity translation

…t to uint16

jcohenadad · 2018-05-20T01:57:36Z

Let's do a comparative test as in #1757 before merging. I'll take care of that and add results in this PR.

jcohenadad

let's wait for the comparative tests before merging

jcohenadad · 2018-05-22T00:25:24Z

Below are the Dice results:

The logs and pickles are under Gdrive/NeuroPoly/Projects/SCT/testing:

results_test_sct_deepseg_sc_180522015441 --> cg_i1740
results_test_sct_deepseg_sc_180521063534 --> master

As expected, cg_i1740 solves the issue on these two problematic datasets:

prague_sct-users_20180518
unknown_sct-users_20180518

However, that should result in at least two fails less, whereas there is only one fail less compared to master, which means that this PR does break compatibility in other datasets. Investigating...

UPDATE 2018-05-22 10.26: When pulling cg_i1740 and trying locally on this subject, a segmentation is created. Maybe there was an issue with branch management on cedar (probably my fault). I'll re-run.

UPDATE 2018-05-22 11.10: I've re-run, making sure cg_i1740 was selected, but there are still blank segmentation on the problematic subjects. Investigating...

UPDATE 2018-05-22 11.18: When running the job in interactive mode, i confirm the running branch is cg_i1740. Investigating further...

UPDATE 2018-05-22 11.24: Realizing now that the modified function was under /spinalcordtoolbox (i though it was only under /scripts), meaning that SCT should have been reinstalled. Nice way to loose 2h...

UPDATE 2018-05-22 14.17: After reinstalling with -p -m -b -d, I still have blank segmentations, as if the re-installation did not successfully update /spinalcordtoolbox modules. Now doing a full installation (with only flag -p) --> THAT WORKED! Anyone has any idea why ./install_sct -p -m -b -d did not update spinalcordtoolbox/centerline but ./install_sct -p did? I though that the line pip install ${SCT_SOURCE} was supposed to update the /spinalcordtoolbox modules? @zougloub do you have any idea? This is not the first time that happened to me (#1702).

UPDATE 2018-05-22 17.09: Updated violin plots and (now with working installation).

charleygros · 2018-05-22T13:52:28Z

@jcohenadad: By comparing the two pickles (master and cg_i1740):

path_cg = '/Users/chgroc/code/i1740/cg_i.pickle'
path_master = '/Users/chgroc/code/i1740/master.pickle'

pickle_file = open(path_cg)
cg_data = pickle.load(pickle_file)
cg_status99 = cg_data[(cg_data.status == 99)]['path_data'].apply(lambda x: x.split('/')[5])

pickle_file = open(path_master)
master_data = pickle.load(pickle_file)
master_status99 = master_data[(master_data.status == 99)]['path_data'].apply(lambda x: x.split('/')[6])

print list(set(set(cg_status99)-set(master_status99)))

we obtain the following result:

['milan_20160628-marcella_DCD-20110907']

I will investigate further this subject

UPDATE 2018-05-22 10.13: I have run locally this subject on the branch cg_i1740 --> all was good: Am I missing something?

UPDATE 2018-05-22 10.17: For this subject (['milan_20160628-marcella_DCD-20110907']), the different status is due to:
On master the dice is: 0.80005518 --> status=0
On cg_i1740 the dice is: 0.79990912 --> status=99

* add a intensity scaling prior to int16 conversion to prevent intensity < 1 * Add refs in the header of the functions * add ref optic deepseg_sc * convert to uint16 * for intensities between 0 and 1 the data was not rescaled, we removed the "if" condition * use changeType * the if condition is crucial for binary images as input. * use skimage.img_as_uint + be robust to negative values with an intensity translation * img_as_int instead of img_as_uint * replace the skimage functions --> rescale in uint16 range then convert to uint16 Former-commit-id: dea98c9

charleygros added 2 commits May 17, 2018 10:20

add a intensity scaling prior to int16 conversion to prevent intensit…

42e9f60

…y < 1

Add refs in the header of the functions

f2ba073

charleygros requested review from jcohenadad and stephaniealley May 17, 2018 14:28

charleygros self-assigned this May 17, 2018

charleygros added the sct_get_centerline context: label May 17, 2018

stephaniealley approved these changes May 17, 2018

View reviewed changes

jcohenadad requested changes May 17, 2018

View reviewed changes

jcohenadad reviewed May 17, 2018

View reviewed changes

jcohenadad added this to the 3.2.0 milestone May 18, 2018

charleygros added 2 commits May 17, 2018 21:19

add ref optic deepseg_sc

9427642

convert to uint16

e1a7cf1

jcohenadad mentioned this pull request May 18, 2018

Output cord segmentation is empty on image with reasonable contrast #1740

Closed

charleygros added 2 commits May 17, 2018 21:54

for intensities between 0 and 1 the data was not rescaled, we removed…

f26a735

… the "if" condition

use changeType

b747c0f

charleygros and others added 5 commits May 18, 2018 12:32

the if condition is crucial for binary images as input.

6d2ed61

use skimage.img_as_uint + be robust to negative values with an intens…

8cd061d

…ity translation

img_as_int instead of img_as_uint

8d69a38

replace the skimage functions --> rescale in uint16 range then conver…

65661c2

…t to uint16

Merge branch 'master' into cg_i1740

41e2e14

jcohenadad approved these changes May 20, 2018

View reviewed changes

jcohenadad requested changes May 20, 2018

View reviewed changes

jcohenadad added 2 commits May 20, 2018 11:05

Merge branch 'master' into cg_i1740

c15dbfe

Merge branch 'master' into cg_i1740

42f9e18

jcohenadad mentioned this pull request May 20, 2018

Create a function that generates a report comparing results (e.g. between SCT versions) #1603

Open

jcohenadad added 3 commits May 21, 2018 22:33

Merge branch 'master' into cg_i1740

d9ebb92

Merge branch 'master' into cg_i1740

c6e6f9d

Merge branch 'master' into cg_i1740

f9694c1

jcohenadad added 2 commits May 22, 2018 12:56

Merge branch 'master' into cg_i1740

84a5437

Merge branch 'master' into cg_i1740

772cbbe

jcohenadad mentioned this pull request May 22, 2018

Testing branches #1702

Closed

jcohenadad approved these changes May 22, 2018

View reviewed changes

jcohenadad merged commit dea98c9 into master May 22, 2018

jcohenadad deleted the cg_i1740 branch May 22, 2018 23:46

jcohenadad added the enhancement category: improves performance/results of an existing feature label May 27, 2018

jcohenadad changed the title ~~Make sct_get_centerline robust to intensities ∈ [0, 1]~~ Make sct_get_centerline robust to intensities with range [0, 1] May 27, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Make sct_get_centerline robust to intensities with range [0, 1] #1746

Make sct_get_centerline robust to intensities with range [0, 1] #1746

charleygros commented May 17, 2018 •

edited

stephaniealley left a comment

jcohenadad left a comment

jcohenadad May 17, 2018

charleygros May 18, 2018

charleygros May 18, 2018

jcohenadad May 18, 2018

jcohenadad May 18, 2018

charleygros May 18, 2018

jcohenadad May 18, 2018

charleygros commented May 18, 2018

jcohenadad commented May 18, 2018 •

edited

charleygros commented May 18, 2018 •

edited by jcohenadad

jcohenadad commented May 18, 2018 •

edited

charleygros commented May 18, 2018 •

edited

jcohenadad commented May 20, 2018

jcohenadad left a comment

jcohenadad commented May 22, 2018 •

edited

charleygros commented May 22, 2018 •

edited

Make sct_get_centerline robust to intensities with range [0, 1] #1746

Make sct_get_centerline robust to intensities with range [0, 1] #1746

Conversation

charleygros commented May 17, 2018 • edited

Requirements

Description of the Change

Steps and Constraints

stephaniealley left a comment

Choose a reason for hiding this comment

jcohenadad left a comment

Choose a reason for hiding this comment

jcohenadad May 17, 2018

Choose a reason for hiding this comment

charleygros May 18, 2018

Choose a reason for hiding this comment

charleygros May 18, 2018

Choose a reason for hiding this comment

jcohenadad May 18, 2018

Choose a reason for hiding this comment

jcohenadad May 18, 2018

Choose a reason for hiding this comment

charleygros May 18, 2018

Choose a reason for hiding this comment

jcohenadad May 18, 2018

Choose a reason for hiding this comment

charleygros commented May 18, 2018

jcohenadad commented May 18, 2018 • edited

charleygros commented May 18, 2018 • edited by jcohenadad

jcohenadad commented May 18, 2018 • edited

charleygros commented May 18, 2018 • edited

jcohenadad commented May 20, 2018

jcohenadad left a comment

Choose a reason for hiding this comment

jcohenadad commented May 22, 2018 • edited

charleygros commented May 22, 2018 • edited

charleygros commented May 17, 2018 •

edited

jcohenadad commented May 18, 2018 •

edited

charleygros commented May 18, 2018 •

edited by jcohenadad

jcohenadad commented May 18, 2018 •

edited

charleygros commented May 18, 2018 •

edited

jcohenadad commented May 22, 2018 •

edited

charleygros commented May 22, 2018 •

edited