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

Detections Metadata #1585

Open
LinasKo opened this issue Oct 9, 2024 · 9 comments · May be fixed by #1589
Open

Detections Metadata #1585

LinasKo opened this issue Oct 9, 2024 · 9 comments · May be fixed by #1589
Assignees
Labels
hacktoberfest Open for contributions during the annual Hacktoberfest event, aimed at encouraging open-source parti help wanted Extra attention is needed

Comments

@LinasKo
Copy link
Collaborator

LinasKo commented Oct 9, 2024

Detections Metadata

Tip

Hacktoberfest is calling! Whether it's your first PR or your 50th, you’re helping shape the future of open source. Help us build the most reliable and user-friendly computer vision library out there! 🌱

This is a summarized version of #1226.
See the original discussion for more context.

In brief: Detections object stores arrays of values and a dict of arrays, each of length N. We'd like to add a global dict of values to store data on a collection-level.

I expect this to be a hard issue, as it involves many elements within the library.


Detections is a class for encoding the results of any model - detection, segmentation, etc. Here's how it looks:

@dataclass
class Detections:
    xyxy: np.ndarray
    mask: Optional[np.ndarray] = None
    confidence: Optional[np.ndarray] = None
    class_id: Optional[np.ndarray] = None
    tracker_id: Optional[np.ndarray] = None
    data: Dict[str, Union[np.ndarray, List]] = field(default_factory=dict)

All of these, as well as data contents are either:

  1. None: Model will never detect anything of that field
  2. empty array: Model detected no elements this time.
  3. Array of N elements: N objects were detected in your image.

What if we want to store 1 value per-list? E.g. a video name for what every detection was extracted from? Or camera parameters?

Let's introduce a new field: metadata. Detections will now look as follows:

@dataclass
class Detections:
    xyxy: np.ndarray
    mask: Optional[np.ndarray] = None
    confidence: Optional[np.ndarray] = None
    class_id: Optional[np.ndarray] = None
    tracker_id: Optional[np.ndarray] = None
    data: Dict[str, Union[np.ndarray, List]] = field(default_factory=dict)
    metadata: Dict[str, Any] = field(default_factory=dict)

The users can set it directly by doing detections.metadata["key"] = "val".

The primary complexity is caused by functions that merge, slice, split and index into detections.

Relevant methods to be updated:

  • __eq__ should use metadata for comparison
  • is_empty should borrow the data for comparison, just like it does with data.
  • __iter__, should NOT return metadata.
  • None of the from_... methods should be affected
  • merge should:
    • retain metadata even when all detections passed were empty.
    • call a new merge_metadata function.
    • merge_data is rather aggressive. It merges if the keys are identical. Let's mimick that here as well - merge if the keys are the same, and the values are identical, while assuming that merge took care of empty detetions.
  • validate_detection_fields should not - there's nothing to check so far.
  • __getitem__ can return either a data element or a sliced detections object. Let's update so it sets the metadata as well.
  • setitem is unaffected.
  • merge_inner_detection_object_pair should merge metadata similarly to how merge does it.
  • JsonSink and CsvSink handle writing detections to files. Let's not touch it yet.

I believe I've covered everything, but we'll check later on. When testing, make sure to test these changes, as well as ByteTrack.update_with_detections and sv.DetectionsSmoother.update_with_detection.


Helpful links:

@LinasKo
Copy link
Collaborator Author

LinasKo commented Oct 9, 2024

Contribution guidelines

If you would like to make a contribution, please check that no one else is assigned already. Then leave a comment such as "Hi, I would like to work on this issue". We're happy to answer any questions about the task even if you choose not to contribute.

Testing

Please share a Google Colab with minimal code to test the new feature. We know it's additional work, but it will speed up the review process. You may use the Starter Template. The reviewer must test each change. Setting up a local environment to do this is time-consuming. Please ensure that Google Colab can be accessed without any issues (make it public). Thank you! 🙏

@LinasKo LinasKo added hacktoberfest Open for contributions during the annual Hacktoberfest event, aimed at encouraging open-source parti help wanted Extra attention is needed labels Oct 9, 2024
@LinasKo
Copy link
Collaborator Author

LinasKo commented Oct 9, 2024

Note: For historical reasons, Detections.empty() and Detections.is_empty() are an exception. When returning frames from a video, for example, the same model may return some defined as array / None when successfully detecting objects, but different fields defined as array / None when nothing is detected. Therefore, empty detections are mostly checked with Detections.is_empty() and treated as a special case.

@souhhmm
Copy link

souhhmm commented Oct 10, 2024

Hey, I'd like to give this issue a try.

@codermks
Copy link

Hey @LinasKo, I would like to work on this issue.

@LinasKo
Copy link
Collaborator Author

LinasKo commented Oct 10, 2024

Hi @souhhmm 👋,

Let's see how it goes. Assigning it to you!

@souhhmm
Copy link

souhhmm commented Oct 10, 2024

Hey @LinasKo, I have a query. Is a detection empty if the metadata is not empty? If so, is there a need to change is_empty at all?
For example,

    empty_detection = sv.Detections(
        xyxy=np.empty((0, 4)),
        data={},
        metadata={"camera_id": 1}
    )

Is this empty?

@LinasKo
Copy link
Collaborator Author

LinasKo commented Oct 10, 2024

Good question! For our purposes, this is empty. It could be that is_empty doesn't need changing after all.

@souhhmm
Copy link

souhhmm commented Oct 10, 2024

Thank you for the clarification. I'll create a PR with a linked colab soon.
Edit: I feel is_empty does require some changes, figuring it out.

@souhhmm souhhmm linked a pull request Oct 10, 2024 that will close this issue
2 tasks
@souhhmm
Copy link

souhhmm commented Oct 11, 2024

Hey @LinasKo, I've created PR #1589, please let me know about any changes/fixes. Looking forward to your feedback!

@LinasKo LinasKo linked a pull request Oct 18, 2024 that will close this issue
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
hacktoberfest Open for contributions during the annual Hacktoberfest event, aimed at encouraging open-source parti help wanted Extra attention is needed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants