Update models #919

dreadatour · 2025-02-11T15:53:38Z

Add validation and tests for the following models:

Bounding box
Pose
Segments

Add conversion to/from different bounding box formats, including normalized versions, for example:

In [1]: from datachain.model.bbox import BBox

In [2]: BBox.from_coco([10, 20, 80, 60])
Out[2]: BBox(title='', coords=[10, 20, 90, 80])

In [3]: BBox.from_coco([0.1, 0.2, 0.8, 0.6], normalized_to=(320, 240))
Out[3]: BBox(title='', coords=[32, 48, 288, 192])

In [4]: BBox.from_list([10, 20, 90, 80]).to_yolo()
Out[4]: [50, 50, 80, 60]

In [5]: BBox.from_list([10, 20, 90, 80]).to_yolo_normalized((100, 100))
Out[5]: [0.5, 0.5, 0.8, 0.6000000000000001]

TODO

update docs: Docs update #921

Add validation and tests for the following models: - Bounding box - Pose - Segments Add conversion to/from different bounding box formats, including normalized versions.

codecov · 2025-02-11T15:59:31Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 87.77%. Comparing base (e523ccd) to head (db9edbe).

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #919      +/-   ##
==========================================
+ Coverage   87.66%   87.77%   +0.11%     
==========================================
  Files         130      131       +1     
  Lines       11698    11804     +106     
  Branches     1592     1596       +4     
==========================================
+ Hits        10255    10361     +106     
  Misses       1043     1043              
  Partials      400      400

Flag	Coverage Δ
datachain	`87.69% <100.00%> (+0.11%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

cloudflare-workers-and-pages · 2025-02-12T04:34:41Z

Deploying datachain-documentation with Cloudflare Pages

Latest commit:	`db9edbe`
Status:	✅ Deploy successful!
Preview URL:	https://8135dee3.datachain-documentation.pages.dev
Branch Preview URL:	https://models-update.datachain-documentation.pages.dev

View logs

shcheklein · 2025-02-13T02:45:15Z

src/datachain/model/pose.py

+        Args:
+            points (Sequence[Sequence[float]]): The x and y coordinates
+                of the keypoints. List of 2 lists: x and y coordinates.
+            normalized_to (Sequence[int], optional): The reference image size


let's just to image size - all tools usually accept size, not normalized_to AFAIK

I was thinking about using img_size as the name of the parameter here, but then it might not be quite clear if we need to define img_size or not and what is the difference here. If normalized_to is not set, then points are absolute pixel coordinates, if normalized_to is set, then points are relative floats within normalized_to image size.

Other possible option might be to separate those methods, such as from_list takes points with absolute coordinates and no image size is required, and from_list_normalized takes relative positions and image size is required.

What do you think in terms of API?

okay, let me even step back - why do we need a such complicated method? that behaves differently depending on the input and the arguments passed? even from this discussion it seems it's too complicated wdyt?

Oh, rhetorical question! 😅 Any function will behaves differently depending on the input and the arguments passed 🙃 Let me describe the way I was thinking.

In this case we have two options:

Split the function into two: from_list + from_list_normalized -> this means we should also have third function to follow the DRY principle, or drop all the validations.

Use one function from_list with additional argument (normalized_to=...)

In terms of API I do not really see the difference, but in terms of "complexity" there is a difference. It is absolutely the same in both cases, except for additional conversion (removing all checks for simplicity here):

# validation points_x, points_y = points if normalized_to is not None: width, height = normalized_to points_x = [coord * width for coord in points_x] points_y = [coord * height for coord in points_y] # processing

It is kind of the same but same time it is complicated and kind of subjective, I would say. I was considering to have two separate functions, but in the end I have decided to make a single one. I still have no strong opinion on this, and if you feel we need to separate these functions, I will be happy to update this PR ❤️

I also have this described in docstrings here. Which also might signalize this was a bad idea, but same time this exactly comment I think we should keep in separate function if we decided to split these.

Any function will behaves differently depending on the input and the arguments passed

in this context specifically - not every. All from_ and to_ are stable to my mind and have precise semantics of an output

Split the function into two: from_list + from_list_normalized -> this means we should also have third function to follow the DRY principle, or drop all the validations.

okay, why do we need from_list at all?

it should be rather from_coco, from_yolo, from_albumentations ...

or it can be from(array, type: Literal['coco', 'yolo', ....])

shcheklein · 2025-02-13T02:45:54Z

src/datachain/model/pose.py

+                "Normalized coordinates must be floats between 0 and 1."
+            )
+            width, height = validate_img_size(normalized_to)
+            points_x = [coord * width for coord in points_x]


can it be a vectorized operation with numpy?

shcheklein · 2025-02-13T02:48:58Z

src/datachain/model/pose.py

+        Returns:
+            Pose: A Pose object.
+        """
+        assert isinstance(points, (tuple, list)), "Pose must be a list of 2 lists."


probably we should be raising proper ValueErrors here, not do asserts

shcheklein · 2025-02-13T03:19:44Z

src/datachain/model/pose.py

+            width, height = validate_img_size(normalized_to)
+            points_x = [coord * width for coord in points_x]
+            points_y = [coord * height for coord in points_y]
+
        return Pose(
            x=[round(coord) for coord in points_x],


same, all these could be vectorized operations most likely

can we outsource it to some exiting lib btw? (I'm afraid that this seemingly simple code has a lot of complexity potentially ... also, AFAIR yolo has some utils and since we depend on it already should we use it?

I will take a look at vectorized operations, thank you for the suggestion 🙏

also, AFAIR yolo has some utils and since we depend on it already should we use it?

I don't want to import ultralytics here because user might not have it installed, and I am not sure we should keep it as an optional dependency. Here I am trying to keep these models dead simple and do not rely on additional optional libraries. These base models (BBox, Pose, Segment`) in general should works with any library, not only Yolo, that's why we do have all these conversions here.

Note

These base models (BBox, Pose, Segment`) in general should works with any library, not only Yolo, that's why we do have all these conversions here.

Strictly saying Pose validation requires exactly 17 points, it is OK for Yolo but may not be OK for other pose models. I think I should remove this validation from here.

shcheklein · 2025-02-13T04:12:07Z

src/datachain/model/bbox.py

+            coords (Sequence[float]): The bounding box coordinates.
+            title (str, optional): The title or label for the bounding box.
+                Defaults to "".
+            normalized_to (Sequence[int], optional): The reference image size


so, does it mean we have the same model that can be normalize and not?

also, even here it is weird ... normalized_to ... the reference image size

shcheklein · 2025-02-13T04:12:59Z

src/datachain/model/bbox.py

+
+        If the input coordinates are normalized (i.e., floats between 0 and 1),
+        they will be converted to absolute pixel values based on the provided
+        image size. The image size should be given as a tuple (width, height)


what is the image size here?

shcheklein · 2025-02-13T04:16:13Z

src/datachain/model/bbox.py

+
+    def to_yolo(self) -> list[int]:
+        """
+        Convert the bounding box to YOLO format.


yolo is normalized by default, no? (at when you train something it asks it to be normalized) - let's refer to utils in albumentations lib to be sure about formats, naming, etc

shcheklein · 2025-02-13T04:19:40Z

src/datachain/model/utils.py

+    assert all(isinstance(value, float) for value in coords), (
+        "Bounding box normalized coordinates must be floats."
+    )
+    assert all(0 <= value <= 1 for value in coords), (


how do bounding boxes behave on the edges (the same with poses) - is it true that it should be always <1 (since coodrinate starts with 0 and is always < image size)?

shcheklein · 2025-02-13T04:20:42Z

src/datachain/model/utils.py

+    width, height = validate_img_size(img_size)
+
+    assert (
+        0 <= coords[0] <= width


same question here - how do they behave on the edges?

Update models

db9edbe

Add validation and tests for the following models: - Bounding box - Pose - Segments Add conversion to/from different bounding box formats, including normalized versions.

dreadatour requested a review from a team February 11, 2025 15:53

dreadatour self-assigned this Feb 11, 2025

dreadatour mentioned this pull request Feb 12, 2025

Docs update #921

Merged

shcheklein reviewed Feb 13, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update models #919

Update models #919

dreadatour commented Feb 11, 2025 •

edited

Loading

codecov bot commented Feb 11, 2025 •

edited

Loading

cloudflare-workers-and-pages bot commented Feb 12, 2025 •

edited

Loading

shcheklein Feb 13, 2025

dreadatour Feb 13, 2025

shcheklein Feb 13, 2025

dreadatour Feb 13, 2025

shcheklein Feb 13, 2025

shcheklein Feb 13, 2025

shcheklein Feb 13, 2025

shcheklein Feb 13, 2025

dreadatour Feb 13, 2025

shcheklein Feb 13, 2025

shcheklein Feb 13, 2025

shcheklein Feb 13, 2025

shcheklein Feb 13, 2025

shcheklein Feb 13, 2025

shcheklein Feb 13, 2025

Update models #919

Are you sure you want to change the base?

Update models #919

Conversation

dreadatour commented Feb 11, 2025 • edited Loading

TODO

codecov bot commented Feb 11, 2025 • edited Loading

Codecov Report

cloudflare-workers-and-pages bot commented Feb 12, 2025 • edited Loading

Deploying datachain-documentation with Cloudflare Pages

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dreadatour commented Feb 11, 2025 •

edited

Loading

codecov bot commented Feb 11, 2025 •

edited

Loading

cloudflare-workers-and-pages bot commented Feb 12, 2025 •

edited

Loading