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

Classification algorithm #76

Merged
merged 9 commits into from Sep 8, 2017

Conversation

Projects
None yet
5 participants
@vessemer
Copy link
Contributor

vessemer commented Aug 27, 2017

Description

The classification algorithm is a deep residual 3D convolutional neural network architecture aimed at feature extraction and dimensionality reduction. It was my solution for the LUNA16 challenge. The method achieved competition performance metric (CPM) of 0.735 and sensitivities of 78.8% and 83.9% at 1 and 4 false positives per scan, respectively. The main benefits are that this algorithm consists of a single architecture in comparison with cumbersome ensemble methods.

Reference to official issue

Feature: Implement classification algorithm #2

Motivation and Context

Usually, it is required by the classification algorithms to preprocess a raw data, (e.g. to fit the voxels' magnitude into unit interval or to resize the CT to the unit dimension). Thus the additional changes related to the load_dicom along with preprocess_dicom has been included and tested as well.

How Has This Been Tested?

Unit tests for trained_model include:

  • Model loading
  • Output consistency

Unit tests for preprocess_dicom:

  • Parameters integrity
  • Preprocessing results for various cases

CLA

  • I have signed the CLA; if other committers are in the commit history, they have signed the CLA as well
import numpy as np
import os
import pytest
import shutil

This comment has been minimized.

@isms

isms Aug 27, 2017

Contributor

Is this used?

This comment has been minimized.

@vessemer

vessemer Aug 27, 2017

Contributor

Thanks for the review, fixed it.

@WGierke

This comment has been minimized.

Copy link
Contributor

WGierke commented Aug 27, 2017

Thanks for your contribution! I'd suggest you to use flake8 to check for proper PEP8 formatting of your code as it will be done by Travis as soon as Travis works again. Here is the output that was generated for me:

➜  concept-to-clinic git:(2_classify_algorithm) flake8 prediction
prediction/classify/trained_model.py:12:20: W291 trailing whitespace
prediction/classify/trained_model.py:18:1: E402 module level import not at top of file
prediction/classify/trained_model.py:18:49: W291 trailing whitespace
prediction/classify/trained_model.py:21:47: W291 trailing whitespace
prediction/classify/trained_model.py:39:82: W291 trailing whitespace
prediction/classify/trained_model.py:64:5: E303 too many blank lines (2)
prediction/classify/src/preprocess_patch.py:18:76: W291 trailing whitespace
prediction/classify/src/preprocess_patch.py:29:31: W291 trailing whitespace
prediction/classify/src/preprocess_patch.py:30:31: W291 trailing whitespace
prediction/classify/src/preprocess_patch.py:31:31: W291 trailing whitespace
prediction/classify/src/preprocess_patch.py:33:86: W291 trailing whitespace
prediction/classify/src/preprocess_patch.py:34:86: W291 trailing whitespace
prediction/classify/src/preprocess_patch.py:37:1: W293 blank line contains whitespace
prediction/classify/src/preprocess_patch.py:45:76: W291 trailing whitespace
prediction/src/tests/test_preprocess_dicom.py:2:1: F401 'os' imported but unused
prediction/src/tests/test_preprocess_dicom.py:4:1: F401 'shutil' imported but unused
prediction/src/tests/test_preprocess_dicom.py:19:5: F841 local variable 'voxel_shape' is assigned to but never used
prediction/src/tests/test_preprocess_dicom.py:39:5: E303 too many blank lines (2)
prediction/src/tests/test_preprocess_dicom.py:41:1: W293 blank line contains whitespace
prediction/src/tests/test_classify_trained_model_predict.py:1:1: F401 'numpy as np' imported but unused
prediction/src/tests/test_classify_trained_model_predict.py:4:1: F401 'shutil' imported but unused
prediction/src/tests/test_classify_trained_model_predict.py:5:1: F401 'glob.glob' imported but unused
prediction/src/tests/test_classify_trained_model_predict.py:12:1: E402 module level import not at top of file
prediction/src/tests/test_classify_trained_model_predict.py:13:1: E402 module level import not at top of file
prediction/src/tests/test_classify_trained_model_predict.py:28:50: W291 trailing whitespace
prediction/src/tests/test_classify_trained_model_predict.py:29:42: W291 trailing whitespace
prediction/src/tests/test_classify_trained_model_predict.py:30:50: W291 trailing whitespace
prediction/src/tests/test_classify_trained_model_predict.py:31:61: W291 trailing whitespace
prediction/src/tests/test_classify_trained_model_predict.py:38:55: W291 trailing whitespace
prediction/src/tests/test_classify_trained_model_predict.py:39:53: W291 trailing whitespace
prediction/src/tests/test_classify_trained_model_predict.py:42:50: W291 trailing whitespace
prediction/src/tests/test_classify_trained_model_predict.py:43:69: W291 trailing whitespace
prediction/src/tests/test_classify_trained_model_predict.py:44:50: W291 trailing whitespace
prediction/src/tests/test_classify_trained_model_predict.py:45:67: W291 trailing whitespace
prediction/src/preprocess/preprocess_dicom.py:13:92: W291 trailing whitespace
prediction/src/preprocess/preprocess_dicom.py:14:62: W291 trailing whitespace
prediction/src/preprocess/preprocess_dicom.py:15:78: W291 trailing whitespace
prediction/src/preprocess/preprocess_dicom.py:16:65: W291 trailing whitespace
prediction/src/preprocess/preprocess_dicom.py:18:80: W291 trailing whitespace
prediction/src/preprocess/preprocess_dicom.py:25:17: E211 whitespace before '('
prediction/src/preprocess/preprocess_dicom.py:25:58: W291 trailing whitespace
prediction/src/preprocess/preprocess_dicom.py:26:70: W291 trailing whitespace
prediction/src/preprocess/preprocess_dicom.py:35:37: W291 trailing whitespace
prediction/src/preprocess/preprocess_dicom.py:41:25: W291 trailing whitespace
prediction/src/preprocess/preprocess_dicom.py:45:90: W291 trailing whitespace
prediction/src/preprocess/preprocess_dicom.py:61:92: W291 trailing whitespace
prediction/src/preprocess/preprocess_dicom.py:62:62: W291 trailing whitespace
prediction/src/preprocess/preprocess_dicom.py:63:78: W291 trailing whitespace
prediction/src/preprocess/preprocess_dicom.py:64:65: W291 trailing whitespace
prediction/src/preprocess/preprocess_dicom.py:66:80: W291 trailing whitespace
prediction/src/preprocess/preprocess_dicom.py:74:5: E303 too many blank lines (2)
prediction/src/preprocess/preprocess_dicom.py:82:5: E303 too many blank lines (2)
prediction/src/preprocess/preprocess_dicom.py:89:1: W293 blank line contains whitespace
prediction/src/preprocess/preprocess_dicom.py:101:78: W291 trailing whitespace
prediction/src/preprocess/load_dicom.py:42:92: W291 trailing whitespace
prediction/src/preprocess/load_dicom.py:58:17: E128 continuation line under-indented for visual indent
prediction/src/preprocess/load_dicom.py:59:1: W293 blank line contains whitespace

You could use formatting tools like the Format option in PyCharm for that or autopep8 for Atom so you don't have to do it manually :)

@vessemer

This comment has been minimized.

Copy link
Contributor

vessemer commented Aug 27, 2017

Thanks for your feedback @WGierke, I think I changed everything accordingly.

@reubano reubano self-assigned this Aug 28, 2017

@isms

This comment has been minimized.

Copy link
Contributor

isms commented Aug 29, 2017

Hi @vessemer, can you please rebase onto the master branch and re-push now that #69 is fixed? (That will resolve the local.yml conflict and we'll also get a clean Travis build.)

@isms isms assigned vessemer and unassigned reubano Aug 31, 2017

@lamby

This comment has been minimized.

Copy link
Contributor

lamby commented Sep 1, 2017

Hi @vessemer Are you having any issues here? Let us know! :)

@vessemer vessemer force-pushed the vessemer:2_classify_algorithm branch from d3e1c28 to 7a9d4da Sep 2, 2017

@vessemer

This comment has been minimized.

Copy link
Contributor

vessemer commented Sep 2, 2017

Hi, @isms, I've done with small fixes and rebase as well.

import sys
import os

sys.path.append(os.path.join(os.path.dirname(__file__), os.path.pardir))

This comment has been minimized.

@lamby

lamby Sep 4, 2017

Contributor

Wait, what? :p

This comment has been minimized.

@vessemer

vessemer Sep 4, 2017

Contributor

As it was mentioned in the technical details from the issue #2:

This feature should be implemented in the prediction/classify/trained_model/predict method. Code to train the model should live in the prediction/classify/src/ folder.
A fully serialized version of the model that can be loaded should live in the prediction/classify/assets/ folder using git-lfs.

The classify shouldn't lie in the prediction/src directory but in the prediction itself. Is there a typo in the issue or is it now outdated?

@vessemer vessemer force-pushed the vessemer:2_classify_algorithm branch from 307101a to e647163 Sep 4, 2017

@lamby

This comment has been minimized.

Copy link
Contributor

lamby commented Sep 6, 2017

Hey all, can we get some clarification who this is blocking on? :) It's assigned to @vessemer but that might not be accurate - please change if so! — lamby

@vessemer vessemer force-pushed the vessemer:2_classify_algorithm branch from e647163 to b462872 Sep 6, 2017

@lamby

This comment has been minimized.

Copy link
Contributor

lamby commented Sep 7, 2017

@vessemer Please could you rebase against master? I might have screwed things up with adding 905ae55 there, so feel free to drop that.

@lamby

This comment has been minimized.

Copy link
Contributor

lamby commented Sep 7, 2017

@vessemer Indeed, there are now 2 .dcm lines in .gitattributes so please drop/fixup that commit when rebasing :)

@vessemer vessemer force-pushed the vessemer:2_classify_algorithm branch from 905ae55 to 3a52220 Sep 7, 2017

@lamby lamby merged commit 507b679 into drivendataorg:master Sep 8, 2017

2 checks passed

concept-to-clinic/cla @vessemer has signed the CLA.
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@lamby

This comment has been minimized.

Copy link
Contributor

lamby commented Sep 8, 2017

Merged, thank you!

@lamby lamby referenced this pull request Sep 8, 2017

Closed

Feature: Implement classification algorithm #2

0 of 2 tasks complete

@vessemer vessemer deleted the vessemer:2_classify_algorithm branch Sep 8, 2017

@reubano reubano referenced this pull request Sep 15, 2017

Closed

small DICOM test images no longer managed in git #116

5 of 5 tasks complete
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment