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.

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

[FR] Add support for loading YOLO annotations with confidences #1460

Closed
2 tasks done
Rusteam opened this issue Dec 7, 2021 · 3 comments
Closed
2 tasks done

[FR] Add support for loading YOLO annotations with confidences #1460

Rusteam opened this issue Dec 7, 2021 · 3 comments
Labels
enhancement Code enhancement

Comments

@Rusteam
Copy link
Contributor

Rusteam commented Dec 7, 2021

Proposal Summary

Load yolo annotations with a confidence parameter

Motivation

  • yolov5 writes a detection confidence to if called with the respective argument: python detect.py ... --save-conf ..
  • If confidence is present as a 6th field in a yolo output txt file, then fiftyone throws an error ValueError: too many values to unpack (expected 5)
  • Confidence is an essential parameter to calculation of main detection metrics such as Average Precision and, hence, mAP which are supported by fiftyone
  • quite easy to achieve by adding an extra argument and a row to _parse_yolo_row inside utils.yolo module.
    However, I'm not sure if it should be added to main Yolo importers and exporters. I'm using load_yolo_annotations function directly.

What areas of FiftyOne does this feature affect?

  • Core: Core fiftyone Python library

Details

Proposed changes inside utils.yolo:

OPTION 1:

def load_yolo_annotations(txt_path, classes, **kwargs): ### Add kwargs to pass into the row parser
    """Loads the YOLO-style annotations from the given TXT file.

    Args:
        txt_path: the path to the annotations TXT file
        classes: the list of class label strings

    Returns:
        a :class:`fiftyone.core.detections.Detections` instance
    """
    detections = []
    for row in _read_file_lines(txt_path):
        detection = _parse_yolo_row(row, classes, **kwargs)  ### Pass the kwargs here
        detections.append(detection)

    return fol.Detections(detections=detections)

def _parse_yolo_row(row, classes, conf=False): ### new conf=False argument
    ### confidence is None if not requested
    if conf:
        (target, xc, yc, w, h), conf = row.split(), None
    else:
        target, xc, yc, w, h, conf = row.split()
    
    try:
        label = classes[int(target)]
    except:
        label = str(target)

    bounding_box = [
        (float(xc) - 0.5 * float(w)),
        (float(yc) - 0.5 * float(h)),
        float(w),
        float(h),
    ]

    return fol.Detection(label=label, bounding_box=bounding_box, confidence=conf) ### add conf to a detection

OPTION 2: alternatively, we can change _parse_yolo_row without any extra kwargs anywhere else:

row_vals = row.split()
if len(row_vals) == 5:
    (target, xc, yc, w, h), conf = row_vals, None
elif len(row_vals) == 6:
    target, xc, yc, w, h, conf = row_vals
else:
    raise NotImplementedError(f"rows with length {len(row_vals)} are not supported")

Willingness to contribute

The FiftyOne Community encourages new feature contributions. Would you or
another member of your organization be willing to contribute an implementation
of this feature?

  • Yes. I can contribute this feature independently.
@Rusteam Rusteam added the enhancement Code enhancement label Dec 7, 2021
@brimoor brimoor changed the title [FR] [FR] Add support for loading YOLO annotations with confidences Dec 7, 2021
@brimoor
Copy link
Contributor

brimoor commented Dec 7, 2021

Hi @Rusteam 👋

I'm onboard with adding support for importing/exporting confidences when working with YOLO format. If you can contribute a PR for this, that'd be awesome! 💪

When importing YOLO labels, I like your option 2: update _parse_yolo_row so that confidence will be included in the Detection objects if this data optionally exists in the TXT files.

For completeness, I'd also like to support exporting confidences when using YOLOv4DatasetExporter and YOLOv5DatasetExporter. In this case, to maintain backwards compatibility, I'd recommend adding a new include_confidence parameter that defaults to False but can be set to True if the user would like to include confidences in the YOLO export. Here's a similar example of this pattern.

@Rusteam
Copy link
Contributor Author

Rusteam commented Dec 8, 2021

That sounds good then, I'll get to it in the coming days

@brimoor
Copy link
Contributor

brimoor commented Dec 26, 2021

#1465

@brimoor brimoor closed this as completed Dec 26, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Code enhancement
Projects
None yet
Development

No branches or pull requests

2 participants