Skip to content
This repository has been archived by the owner on Feb 22, 2020. It is now read-only.

#271 Classification Evaluation #290

Merged
merged 2 commits into from
Jan 23, 2018

Conversation

WGierke
Copy link
Contributor

@WGierke WGierke commented Jan 19, 2018

I'm currently adding the possibility to evaluate a classification model based on the LIDC dataset as described in #271. Furthermore, I'm currently benchmarking the model that's implemented at the moment.
Even if I still haven't finished, I wanted to show you what I'm currently working at.

CLA

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

nodule_list = []
for annotation in scan.annotations:
centroid_x, centroid_y, centroid_z = annotation.centroid()
z_index = int((centroid_z - min_z) / scan.slice_thickness)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This step should maybe be transferred to an other location where it can also be tested separately. Computing the slice index from the relative image positions still feels a bit hacky.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Serhiy-Shekhovtsov @vessemer Do you have experience with getting the slice index from the ImagePositient information? The DICOM images have ImagePosition information (z_axis) from e.g. -435 to -63. The nodule has one of -252 so we're required to calculate the slice index (here: 61) out of that. Do you know whether that's the right way or whether we are already doing this somewhere?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this what you are looking for?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The offset then used here to convert the coordinates

    coord = np.rint((coord - np.array(origin)) * np.array(spacing))

Copy link
Contributor

@vessemer vessemer Jan 25, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@WGierke, @Serhiy-Shekhovtsov, sorry, I've missed that, it should be np.rint((coord - np.array(origin)) / np.array(spacing)), also I didn't get for what reason, @Serhiy-Shekhovtsov, you remove this line, I should missing something :/

@WGierke
Copy link
Contributor Author

WGierke commented Jan 20, 2018

The current implementation takes 8.5h to run, gives an average accuracy/precision of 10% and an average loss of 3.55 :/

return -np.log(1 - p)


def evaluate_classification(model_path=None):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will it be better to make evaluation functionality console executable with appropriate metrics and directories arguments? Mean, that this is a bit out of user usage.
I'd also suggest to put all metrics in a different file.

CONFIDENCE_THRESHOLD = 0.5


def get_accuracy(TP, TN, FP, FN):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

would you be able to use more descriptive variable names?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure. Do you maybe see any major flaws in the implemented logic of the whole file? I'm just asking because an accuracy of 10% seems pretty low to me.

@WGierke WGierke changed the title [WIP] Classification Evaluation #271 Classification Evaluation Jan 22, 2018
@reubano
Copy link
Contributor

reubano commented Jan 23, 2018

It looks good, and no I don't see any major flaws. If someone with more experience in applying these metrics to CT scans has suggestions he/she can always improve this with future PRs. It also seems like the sys.path.insert... isn't making Travis happy. I'll see what can be done about that.

@reubano
Copy link
Contributor

reubano commented Jan 23, 2018

This is what I got...

src/algorithms/evaluation/evaluation.py

import os

import numpy as np
import pylidc as pl

try:
    from ....config import Config
except ValueError:
    from config import Config

...

src/tests/test_evaluate_classification.py

from ..algorithms.evaluation.evaluation import evaluate_classification


def test_evaluate_classification(model_path=None):
    assert evaluate_classification(model_path)

run with docker-compose -f local.yml run prediction pytest src/tests/test_evaluate_classification.py

I believe you'll also have to add tdmq to the requirements and fix whatever remaining style errors you may see via flake8 prediction

@WGierke
Copy link
Contributor Author

WGierke commented Jan 23, 2018

@reubano Done. Thanks!

@lamby
Copy link
Contributor

lamby commented Jan 23, 2018

Awesome :)

@lamby lamby merged commit 57452ab into drivendataorg:master Jan 23, 2018
@vessemer vessemer mentioned this pull request Jan 23, 2018
1 task
@WGierke WGierke mentioned this pull request Jan 25, 2018
1 task
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants