169 | 4 MetaData and PreprocessCT refactored, grt123 processing adopted #170

vessemer · 2017-10-17T06:27:54Z

Refactored classes PreprocessCT and MetaData. Preprocessing of grt123 algorithm has been replaced by them.

Description

PreprocessCT is now a subclass of Params, both are documented along with MetaData.
Correct me if I've mistaken, but there is a bug in the processing part of the grt123 prediction algorithm adoptation:

image = sitk.GetArrayFromImage(image_itk)
spacing = np.array(image_itk.GetSpacing())[::-1]  
image = lum_trans(image)
image = resample(image, spacing, np.array([1, 1, 1]), order=1)[0]

spacing will became redundant after re-sampling.

for nodule in nodule_list:
    print(nodule)
    nod_location = np.array([np.float32(nodule[s]) for s in ["z", "y", "x"]])
    nod_location *= spacing

estimation of the nod_location should also take into account both the origin of the CT scan and new_spacing = [1, 1, 1] and therefore shal looks like

nod_location = (nod_location - origin) / new_spacing

Neglection of the origin and new_spacing leads to estimation of a complitely different CT part.

the old estimation of patches by SimpleCrop requires np.pad to be applied on each patch independently for the same CT scan.

The aforementioned bugs were corrected along with the adoptation of grt123 preprocessing via preprocess_ct.py

Reference to official issue

This addresses #169 and partially covers #4

How Has This Been Tested?

For the grt123_preprocess, all tests lie in test_grt123_preprocess and are written in order to ensure fully interchangeability.

CLA

I have signed the CLA; if other committers are in the commit history, they have signed the CLA as well

WGierke · 2017-10-17T06:29:46Z

prediction/src/algorithms/classify/src/gtr123_model.py

@@ -288,23 +242,17 @@ def predict(image_itk, nodule_list, model_path="src/algorithms/classify/assets/g

    if torch.cuda.is_available():
        casenet = torch.nn.DataParallel(casenet).cuda()
-    # else:
+        # else:


I'm not sure if the indentation level is now correct?

lamby · 2017-10-17T13:11:39Z

prediction/src/preprocess/crop_patches.py

@@ -21,10 +21,12 @@ def mm2voxel(coord, origin=0., spacing=1.):
    origin = scipy.ndimage._ni_support._normalize_sequence(origin, len(coord))
    spacing = scipy.ndimage._ni_support._normalize_sequence(spacing, len(coord))
    coord = np.ceil(coord - np.array(origin)) / np.array(spacing)
+    print(coord)


Committed by accident?

lamby · 2017-10-17T13:12:46Z

prediction/src/preprocess/preprocess_ct.py

 import numpy as np
 import scipy.ndimage
 from src.preprocess import load_ct


 class Params:
-    """Params for CT data preprocessing.
+    """Params for CT data pre-processing.


Please try and make these sorts of changes in a separate commit in future.. much easier to review :)

Ok, thank you for the review :)

lamby · 2017-10-17T13:13:07Z

prediction/src/preprocess/preprocess_ct.py

@@ -52,61 +59,81 @@ def __init__(self, hu_transform=False, clip_lower=None, clip_upper=None,
            raise ValueError('The min_max_normalize should be bool or int')
        self.min_max_normalize = min_max_normalize

+        if not isinstance(scale, (int, float)) and (scale is not None):
+            raise ValueError('The scale should be float or int')


TypeError would seem more suitable here.

Indeed it is.

lamby · 2017-10-17T13:13:13Z

prediction/src/preprocess/preprocess_ct.py


-class PreprocessCT:
+        if dtype not in np.typeDict.keys() and (dtype is not None):
+            raise ValueError('The dtype should be one of appropriate np.dtype')


lamby · 2017-10-17T13:14:43Z

prediction/src/tests/test_load_ct.py

    assert all([m_axis == o_axis for m_axis, o_axis in zipped])

    meta = load_ct.load_ct(metaimage_path, voxel=False)
-    spacing = list(reversed(meta.GetSpacing()))
+    spacing = meta.GetSpacing()[::-1]


The SimpleITK reads CT scan array from MetaImages in zyx order, methods like GetSpacing | GetOrigin return values in xyz order, though.

Can you add this as a comment in the code? :)

lamby · 2017-10-18T12:33:38Z

prediction/src/tests/test_grt123_preprocess.py

+        self.stride = 4
+        self.filling_value = 160
+
+    def __call__(self, imgs, target):


Kinda gross to abuse dunderscore call. I'm assuming this code is from elsewhere?

Yes, this code had been taken from the edited version of the original SimpleCrop class. I've put unchanged functions into test_grt123_preprocess.py in order to ensure absolute interchangeability.

lamby · 2017-10-19T13:11:19Z

Thanks!

vessemer changed the title ~~169 MetaData and PreprocessCT refactored 4 grt123~~ 169 | 4 MetaData and PreprocessCT refactored, grt123 processing adopted Oct 17, 2017

WGierke reviewed Oct 17, 2017

View reviewed changes

vessemer force-pushed the 169_metadata_refactoring branch 2 times, most recently from 1c51d07 to 92addb1 Compare October 17, 2017 07:21

lamby suggested changes Oct 17, 2017

View reviewed changes

vessemer force-pushed the 169_metadata_refactoring branch 7 times, most recently from 14543c3 to 389824f Compare October 18, 2017 03:06

lamby reviewed Oct 18, 2017

View reviewed changes

vessemer added 2 commits October 19, 2017 13:23

Refactored CT preprocess

6c20d0d

grt123 processing adaptation

1acd7c7

vessemer force-pushed the 169_metadata_refactoring branch from 389824f to 1acd7c7 Compare October 19, 2017 11:23

lamby merged commit 81972cc into drivendataorg:master Oct 19, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

169 | 4 MetaData and PreprocessCT refactored, grt123 processing adopted #170

169 | 4 MetaData and PreprocessCT refactored, grt123 processing adopted #170

vessemer commented Oct 17, 2017 •

edited

Loading

WGierke Oct 17, 2017

vessemer Oct 17, 2017

lamby Oct 17, 2017

vessemer Oct 18, 2017

lamby Oct 17, 2017

vessemer Oct 18, 2017

lamby Oct 17, 2017

vessemer Oct 18, 2017

lamby Oct 17, 2017

lamby Oct 17, 2017

vessemer Oct 18, 2017

lamby Oct 18, 2017

lamby Oct 18, 2017

vessemer Oct 18, 2017

lamby commented Oct 19, 2017

169 | 4 MetaData and PreprocessCT refactored, grt123 processing adopted #170

169 | 4 MetaData and PreprocessCT refactored, grt123 processing adopted #170

Conversation

vessemer commented Oct 17, 2017 • edited Loading

Description

Reference to official issue

How Has This Been Tested?

CLA

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

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

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lamby commented Oct 19, 2017

vessemer commented Oct 17, 2017 •

edited

Loading