Skip to content
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.

Sign up for GitHub

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

Fixing #5254 #5267

Merged
merged 2 commits into from
Dec 13, 2024
Merged

Fixing #5254 #5267

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
686be45
fix #5254
brimoor Dec 13, 2024
d41fffc
TP/FP/NP support for binary classification model evaluation
imanjra Dec 13, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
58 changes: 31 additions & 27 deletions fiftyone/operators/builtins/panels/model_evaluation/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,16 +5,17 @@
| `voxel51.com <https://voxel51.com/>`_
|
"""

from collections import defaultdict, Counter
import os
import traceback
import fiftyone.operators.types as types

from collections import defaultdict, Counter
import numpy as np

from fiftyone import ViewField as F
from fiftyone.operators.categories import Categories
from fiftyone.operators.panel import Panel, PanelConfig
from fiftyone.core.plots.plotly import _to_log_colorscale
import fiftyone.operators.types as types


STORE_NAME = "model_evaluation_panel_builtin"
Expand Down Expand Up @@ -104,29 +105,32 @@ def get_avg_confidence(self, per_class_metrics):
total += metrics["confidence"]
return total / count if count > 0 else None

def get_tp_fp_fn(self, ctx):
view_state = ctx.panel.get_state("view") or {}
key = view_state.get("key")
dataset = ctx.dataset
tp_key = f"{key}_tp"
fp_key = f"{key}_fp"
fn_key = f"{key}_fn"
tp_total = (
sum(ctx.dataset.values(tp_key))
if dataset.has_field(tp_key)
else None
)
fp_total = (
sum(ctx.dataset.values(fp_key))
if dataset.has_field(fp_key)
else None
)
fn_total = (
sum(ctx.dataset.values(fn_key))
if dataset.has_field(fn_key)
else None
)
return tp_total, fp_total, fn_total
def get_tp_fp_fn(self, info, results):
# Binary classification
imanjra marked this conversation as resolved.
Show resolved Hide resolved
if (
info.config.type == "classification"
and info.config.method == "binary"
):
neg_label, pos_label = results.classes
tp_count = np.count_nonzero(
(results.ytrue == pos_label) & (results.ypred == pos_label)
)
fp_count = np.count_nonzero(
(results.ytrue != pos_label) & (results.ypred == pos_label)
)
fn_count = np.count_nonzero(
(results.ytrue == pos_label) & (results.ypred != pos_label)
)
return tp_count, fp_count, fn_count

# Object detection
if info.config.type == "detection":
tp_count = np.count_nonzero(results.ytrue == results.ypred)
fp_count = np.count_nonzero(results.ytrue == results.missing)
fn_count = np.count_nonzero(results.ypred == results.missing)
return tp_count, fp_count, fn_count
Comment on lines +131 to +135
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Possible miscalculation of false positives and false negatives in object detection

In the object detection case, the calculation of fp_count and fn_count might be incorrect. The fp_count should count cases where ypred is present but ytrue is missing (i.e., predictions with no corresponding ground truth). Similarly, fn_count should count cases where ytrue is present but ypred is missing (i.e., ground truth objects missed by predictions). The current implementation counts fp_count as np.count_nonzero(results.ytrue == results.missing), which does not consider whether ypred is present.

To address this issue, consider modifying the calculations as follows:

 def get_tp_fp_fn(self, info, results):
     # Object detection
     if info.config.type == "detection":
-        tp_count = np.count_nonzero(results.ytrue == results.ypred)
-        fp_count = np.count_nonzero(results.ytrue == results.missing)
-        fn_count = np.count_nonzero(results.ypred == results.missing)
+        tp_count = np.count_nonzero(
+            (results.ytrue != results.missing) & 
+            (results.ypred != results.missing) & 
+            (results.ytrue == results.ypred)
+        )
+        fp_count = np.count_nonzero(
+            (results.ytrue == results.missing) & 
+            (results.ypred != results.missing)
+        )
+        fn_count = np.count_nonzero(
+            (results.ytrue != results.missing) & 
+            (results.ypred == results.missing)
+        )
         return tp_count, fp_count, fn_count
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if info.config.type == "detection":
tp_count = np.count_nonzero(results.ytrue == results.ypred)
fp_count = np.count_nonzero(results.ytrue == results.missing)
fn_count = np.count_nonzero(results.ypred == results.missing)
return tp_count, fp_count, fn_count
if info.config.type == "detection":
tp_count = np.count_nonzero(
(results.ytrue != results.missing) &
(results.ypred != results.missing) &
(results.ytrue == results.ypred)
)
fp_count = np.count_nonzero(
(results.ytrue == results.missing) &
(results.ypred != results.missing)
)
fn_count = np.count_nonzero(
(results.ytrue != results.missing) &
(results.ypred == results.missing)
)
return tp_count, fp_count, fn_count


return None, None, None

def get_map(self, results):
try:
Expand Down Expand Up @@ -298,7 +302,7 @@ def load_evaluation(self, ctx):
per_class_metrics
)
metrics["tp"], metrics["fp"], metrics["fn"] = self.get_tp_fp_fn(
ctx
info, results
)
metrics["mAP"] = self.get_map(results)
evaluation_data = {
Expand Down
Loading