[BUG] evaluate_detections() does not allow samples with None-valued ground truth

luke-iqt commented 3 years ago

System information

OS Platform and Distribution (e.g., Linux Ubuntu 16.04): Ubuntu 20.04
FiftyOne installed from (pip or source): pip
FiftyOne version (run fiftyone --version): fiftyone v0.9.3, Voxel51, Inc.
Python version: Python 3.6.9

Commands to reproduce

As thoroughly as possible, please provide the Python and/or shell commands used to encounter the issue. Application steps can be described in the next section.

view = dataset.match_tags("eval")
view.evaluate_detections(    "914_40k_predict", gt_field="detections", eval_key="eval")

Describe the problem

The evaluate_detections() expects the ground truth field to be present on every sample and have detections. However, I am working with a dataset where the object I am trying to detect is rare. I would like to have images without the object to try and better evaluate false positives. When the evaluate_detections() function runs into a sample that has the gt_field set to None it crashes with the following error:

Evaluating detections...
   1% |/----------------|   3/485 [79.8ms elapsed, 12.8s remaining, 37.6 samples/s] 
---------------------------------------------------------------------------
AttributeError                            Traceback (most recent call last)
<ipython-input-51-721d08e0d620> in <module>
      3 print(view.skip(3).limit(1).first())
      4 view.evaluate_detections(
----> 5     "914_40k_predict", gt_field="detections", eval_key="eval"
      6 )
      7 

/usr/local/lib/python3.6/dist-packages/fiftyone/core/collections.py in evaluate_detections(self, pred_field, gt_field, eval_key, classes, missing, method, iou, classwise, config, **kwargs)
   1614             classwise=classwise,
   1615             config=config,
-> 1616             **kwargs,
   1617         )
   1618 

/usr/local/lib/python3.6/dist-packages/fiftyone/utils/eval/detection.py in evaluate_detections(samples, pred_field, gt_field, eval_key, classes, missing, method, iou, classwise, config, **kwargs)
    136             for image in images:
    137                 image_matches = eval_method.evaluate_image(
--> 138                     image, eval_key=eval_key
    139                 )
    140                 matches.extend(image_matches)

/usr/local/lib/python3.6/dist-packages/fiftyone/utils/eval/coco.py in evaluate_image(self, sample_or_frame, eval_key)
    133             preds = _copy_detections(preds)
    134 
--> 135         return _coco_evaluation_single_iou(gts, preds, eval_key, self.config)
    136 
    137     def generate_results(

/usr/local/lib/python3.6/dist-packages/fiftyone/utils/eval/coco.py in _coco_evaluation_single_iou(gts, preds, eval_key, config)
    420 
    421     cats, pred_ious, iscrowd = _coco_evaluation_setup(
--> 422         gts, preds, [id_key], iou_key, config
    423     )
    424 

/usr/local/lib/python3.6/dist-packages/fiftyone/utils/eval/coco.py in _coco_evaluation_setup(gts, preds, id_keys, iou_key, config, max_preds)
    478         cats[label]["preds"].append(det)
    479 
--> 480     for det in gts.detections:
    481         det[iou_key] = _NO_MATCH_IOU
    482         for id_key in id_keys:

AttributeError: 'NoneType' object has no attribute 'detections'

The sample it failed on is:

view = dataset.match_tags("eval")
print(view.skip(3).limit(1).first())
view.evaluate_detections(
    "914_40k_predict", gt_field="detections", eval_key="eval"
)
# Evaluate `predictions` w.r.t. labels in `ground_truth` field
view = dataset.match_tags("eval")
print(view.skip(3).limit(1).first())
view.evaluate_detections(
    "914_40k_predict", gt_field="detections", eval_key="eval"
)

session = fo.launch_app(view)

# Convert to evaluation patches
eval_patches = view.to_evaluation_patches("eval")
print(eval_patches)

# View patches in the App
session.view = eval_patches
<SampleView: {
    'id': '60a3bf27f3610f2b7a82541b',
    'media_type': 'image',
    'filepath': '/tf/media/capture-5-13/Boeing 737-832/a4417d_193_54_14213_2021-05-13-12-02-17.jpg',
    'tags': BaseList(['capture-5-13', 'eval']),
    'metadata': <ImageMetadata: {
        'size_bytes': 892032,
        'mime_type': 'image/jpeg',
        'width': 1920,
        'height': 1080,
        'num_channels': 3,
    }>,
    'external_id': <Classification: {
        'id': '60a3bf27f3610f2b7a825416',
        'tags': BaseList([]),
        'label': 'a4417d_193_54_14213_2021-05-13-12-02-17',
        'confidence': None,
        'logits': None,
    }>,
    'bearing': <Classification: {
        'id': '60a3bf27f3610f2b7a825417',
        'tags': BaseList([]),
        'label': '193',
        'confidence': None,
        'logits': None,
    }>,
    'elevation': <Classification: {
        'id': '60a3bf27f3610f2b7a825418',
        'tags': BaseList([]),
        'label': '54',
        'confidence': None,
        'logits': None,
    }>,
    'distance': <Classification: {
        'id': '60a3bf27f3610f2b7a825419',
        'tags': BaseList([]),
        'label': '14213',
        'confidence': None,
        'logits': None,
    }>,
    'icao24': <Classification: {
        'id': '60a3bf27f3610f2b7a82541a',
        'tags': BaseList([]),
        'label': 'a4417d',
        'confidence': None,
        'logits': None,
    }>,
    'model': <Classification: {
        'id': '60d5e6fd5e08d80243cb6060',
        'tags': BaseList([]),
        'label': '737-832',
        'confidence': None,
        'logits': None,
    }>,
    'manufacturer': <Classification: {
        'id': '60d5e6fd5e08d80243cb6061',
        'tags': BaseList([]),
        'label': 'Boeing',
        'confidence': None,
        'logits': None,
    }>,
    'norm_model': <Classification: {
        'id': '60d5e7b45e08d80243ce2017',
        'tags': BaseList([]),
        'label': '737-800',
        'confidence': None,
        'logits': None,
    }>,
    'labelbox_id': 'ckqtxl73g14dy0y7mg4ar0fqh',
    'detections': None,
    'operatorcallsign': <Classification: {
        'id': '60d5e6fd5e08d80243cb6062',
        'tags': BaseList([]),
        'label': 'DELTA',
        'confidence': None,
        'logits': None,
    }>,
    '914_40k_predict': <Detections: {'detections': BaseList([])}>,
    '914_40k_predict_full': <Detections: {'detections': BaseList([])}>,
    'eval_tp': None,
    'eval_fp': None,
    'eval_fn': None,
}>

What areas of FiftyOne does this bug affect?

[ ] App: FiftyOne application issue
[ X] Core: Core fiftyone Python library issue
[ ] Server: Fiftyone server issue

Willingness to contribute

The FiftyOne Community encourages bug fix contributions. Would you or another member of your organization be willing to contribute a fix for this bug to the FiftyOne codebase?

[ ] Yes. I can contribute a fix for this bug independently.
[ X] Yes. I would be willing to contribute a fix for this bug with guidance from the FiftyOne community.
[ ] No. I cannot contribute a bug fix at this time.

brimoor commented 3 years ago

Hi @luke-iqt 👋

I'd argue that not allowing None-valued ground truth labels is a feature, not a bug 😄

The rationale is that None means "missing", while an empty Detections() means "was included in labeling efforts but nothing was present".

My suggestion would be to either:

use dataset.exists("detections") to grab a view that contains only samples with non-None ground truth, and then evaluate on that
replace None-valued ground truths with Detections(), and then you can evaluate on the entire dataset

luke-iqt commented 3 years ago

That is a great point @brimoor 🙌

I will update my detection import process so that samples that don't get any labeled Detections will have an empty one assigned. I am using the LabelBox Import function... it would be interesting to add an optional argument to the import_from_labelbox() function that would create an empty Detections or Classification for samples that get returned without one. Here is an example of a JSON item from a LabelBox export that doesn't have any Labels:

{
  "ID": "ckou3mobp00003c5si8rptn43",
  "DataRow ID": "ckou3j0qe9zix0yasg4zbai6i",
  "Labeled Data": "https://storage.labelbox.com/",
  "Label": {},
  "Created By": "lberndt@iqt.org",
  "Project Name": "jsm-test",
  "Created At": "2021-05-18T13:55:40.000Z",
  "Updated At": "2021-05-18T13:55:53.000Z",
  "Seconds to Label": 7.335000000000001,
  "External ID": null,
  "Agreement": -1,
  "Benchmark Agreement": -1,
  "Benchmark ID": null,
  "Dataset Name": "jsm-test",
  "Reviews": [],
  "View Label": "https://editor.labelbox.com?project=",
  "Has Open Issues": 0,
  "Skipped": true
}

In case it is useful for anyone, here is a little snippet for adding an empty detections to the samples in a view:

view = dataset.match_tags("eval")
for sample in view:
    if sample["detections"] == None:
        sample["detections"] = fo.Detections(detections=[])
        sample.save()

brimoor commented 3 years ago

@luke-iqt ah I see, thanks for sharing more details about your import workflow. I like your suggestion for handling None vs empty when importing from annotation vendors: https://github.com/voxel51/fiftyone/issues/1113.

voxel51 / fiftyone