-
Notifications
You must be signed in to change notification settings - Fork 146
#271 Classification Evaluation #290
#271 Classification Evaluation #290
Conversation
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) |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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))
There was a problem hiding this comment.
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 :/
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): |
There was a problem hiding this comment.
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): |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
94b5d81
to
1af3e57
Compare
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 |
This is what I got...
import os
import numpy as np
import pylidc as pl
try:
from ....config import Config
except ValueError:
from config import Config
...
from ..algorithms.evaluation.evaluation import evaluate_classification
def test_evaluate_classification(model_path=None):
assert evaluate_classification(model_path) run with I believe you'll also have to add |
@reubano Done. Thanks! |
Awesome :) |
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