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

Update models #919

Closed
wants to merge 1 commit into from
Closed

Update models #919

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
358 changes: 319 additions & 39 deletions src/datachain/model/bbox.py
View file
Open in desktop

Large diffs are not rendered by default.

131 changes: 127 additions & 4 deletions src/datachain/model/pose.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,12 @@
from collections.abc import Sequence
from typing import Optional

from pydantic import Field

from datachain.lib.data_model import DataModel

from .utils import validate_img_size


class Pose(DataModel):
"""
Expand All @@ -19,22 +24,76 @@ class Pose(DataModel):
y: list[int] = Field(default=[])

@staticmethod
def from_list(points: list[list[float]]) -> "Pose":
def from_list(
points: Sequence[Sequence[float]],
normalized_to: Optional[Sequence[int]] = None,
) -> "Pose":
"""
Create a Pose instance from a list of x and y coordinates.

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)
via the `normalized_to` argument.

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
Copy link
Member

Choose a reason for hiding this comment

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

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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?

Copy link
Member

Choose a reason for hiding this comment

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

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?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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:

  1. 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.
  2. 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.

Copy link
Member

Choose a reason for hiding this comment

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

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', ....])

(width, height) for denormalizing the bounding box. If None (default),
the coordinates are assumed to be absolute pixel values.

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

Choose a reason for hiding this comment

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

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

assert len(points) == 2, "Pose must be a list of 2 lists: x and y coordinates."
points_x, points_y = points
assert isinstance(points_x, (tuple, list)) and isinstance(
points_y, (tuple, list)
), "Pose must be a list of 2 lists."
assert len(points_x) == len(points_y) == 17, (
"Pose x and y coordinates must have the same length of 17."
)
assert all(
isinstance(value, (int, float)) for value in [*points_x, *points_y]
), "Pose coordinates must be floats or integers."

if normalized_to is not None:
assert all(0 <= coord <= 1 for coord in [*points_x, *points_y]), (
"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]
Copy link
Member

Choose a reason for hiding this comment

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

can it be a vectorized operation with numpy?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Just wonder if it will be faster for the list of 17 items 🤔

points_y = [coord * height for coord in points_y]

return Pose(
x=[round(coord) for coord in points_x],
Copy link
Member

Choose a reason for hiding this comment

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

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?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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.

y=[round(coord) for coord in points_y],
)

def to_normalized(self, img_size: Sequence[int]) -> tuple[list[float], list[float]]:
"""
Return the pose keypoints in normalized coordinates.

Normalized coordinates are floats between 0 and 1, representing the
relative position of the pixels in the image.

Returns:
tuple[list[float], list[float]]: The pose keypoints
with normalized coordinates.
"""
width, height = validate_img_size(img_size)
assert all(x <= width and y <= height for x, y in zip(self.x, self.y)), (
"Pose keypoints are out of image size."
)
return (
[coord / width for coord in self.x],
[coord / height for coord in self.y],
)

@staticmethod
def from_dict(points: dict[str, list[float]]) -> "Pose":
def from_dict(points: dict[str, Sequence[float]]) -> "Pose":
assert isinstance(points, dict) and set(points) == {
"x",
"y",
Expand All @@ -60,22 +119,86 @@ class Pose3D(DataModel):
visible: list[float] = Field(default=[])

@staticmethod
def from_list(points: list[list[float]]) -> "Pose3D":
def from_list(
points: Sequence[Sequence[float]],
normalized_to: Optional[Sequence[int]] = None,
) -> "Pose3D":
"""
Create a Pose3D instance from a list of x, y coordinates and visibility values.

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)
via the `normalized_to` argument.

Args:
points (Sequence[Sequence[float]]): The x and y coordinates
of the keypoints. List of 3 lists: x, y coordinates
and visibility values.
normalized_to (Sequence[int], optional): The reference image size
(width, height) for denormalizing the bounding box. If None (default),
the coordinates are assumed to be absolute pixel values.

Returns:
Pose3D: A Pose3D object.

"""
assert isinstance(points, (tuple, list)), (
"Pose3D must be a tuple or list of 3 lists."
)
assert len(points) == 3, (
"Pose3D must be a list of 3 lists: x, y coordinates and visible."
)
points_x, points_y, points_v = points
assert (
isinstance(points_x, (tuple, list))
and isinstance(points_y, (tuple, list))
and isinstance(points_v, (tuple, list))
), "Pose3D must be a tuple or list of 3 lists."
assert len(points_x) == len(points_y) == len(points_v) == 17, (
"Pose3D x, y coordinates and visible must have the same length of 17."
)
assert all(
isinstance(value, (int, float))
for value in [*points_x, *points_y, *points_v]
), "Pose3D coordinates must be floats or integers."

if normalized_to is not None:
assert all(0 <= coord <= 1 for coord in [*points_x, *points_y]), (
"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]
points_y = [coord * height for coord in points_y]

return Pose3D(
x=[round(coord) for coord in points_x],
y=[round(coord) for coord in points_y],
visible=points_v,
visible=list(points_v),
)

def to_normalized(
self,
img_size: Sequence[int],
) -> tuple[list[float], list[float], list[float]]:
"""
Return the pose 3D keypoints in normalized coordinates.

Normalized coordinates are floats between 0 and 1, representing the
relative position of the pixels in the image.

Returns:
tuple[list[float], list[float], list[float]]: The pose keypoints
with normalized coordinates and visibility values.
"""
width, height = validate_img_size(img_size)
assert all(x <= width and y <= height for x, y in zip(self.x, self.y)), (
"Pose3D keypoints are out of image size."
)
return (
[coord / width for coord in self.x],
[coord / height for coord in self.y],
self.visible,
)

@staticmethod
Expand Down
62 changes: 61 additions & 1 deletion src/datachain/model/segment.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,12 @@
from collections.abc import Sequence
from typing import Optional

from pydantic import Field

from datachain.lib.data_model import DataModel

from .utils import validate_img_size


class Segment(DataModel):
"""
Expand All @@ -21,23 +26,78 @@ class Segment(DataModel):
y: list[int] = Field(default=[])

@staticmethod
def from_list(points: list[list[float]], title: str = "") -> "Segment":
def from_list(
points: Sequence[Sequence[float]],
title: str = "",
normalized_to: Optional[Sequence[int]] = None,
) -> "Segment":
"""
Create a Segment object from a list of x and y coordinates.

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)
via the `normalized_to` argument.

Args:
points (Sequence[Sequence[float]]): The x and y coordinates
of the keypoints. List of 2 lists: x and y coordinates.
title (str, optional): The title or label for the segment. Defaults to "".
normalized_to (Sequence[int], optional): The reference image size
(width, height) for denormalizing the bounding box. If None (default),
the coordinates are assumed to be absolute pixel values.

Returns:
Segment: A Segment object.
"""
assert isinstance(points, (tuple, list)), "Segment must be a list of 2 lists."
assert len(points) == 2, (
"Segment must be a list of 2 lists: x and y coordinates."
)
points_x, points_y = points
assert isinstance(points_x, (tuple, list)) and isinstance(
points_y, (tuple, list)
), "Segment must be a list of 2 lists."
assert len(points_x) == len(points_y), (
"Segment x and y coordinates must have the same length."
)
assert all(
isinstance(value, (int, float)) for value in [*points_x, *points_y]
), "Segment coordinates must be floats or integers."

if normalized_to is not None:
assert all(0 <= coord <= 1 for coord in [*points_x, *points_y]), (
"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]
points_y = [coord * height for coord in points_y]

return Segment(
title=title,
x=[round(coord) for coord in points_x],
y=[round(coord) for coord in points_y],
)

def to_normalized(self, img_size: Sequence[int]) -> tuple[list[float], list[float]]:
"""
Return the segment in normalized coordinates.

Normalized coordinates are floats between 0 and 1, representing the
relative position of the pixels in the image.

Returns:
tuple[list[float], list[float]]: The segment with normalized coordinates.
"""
width, height = validate_img_size(img_size)
assert all(x <= width and y <= height for x, y in zip(self.x, self.y)), (
"Segment keypoints are out of image size."
)
return (
[coord / width for coord in self.x],
[coord / height for coord in self.y],
)

@staticmethod
def from_dict(points: dict[str, list[float]], title: str = "") -> "Segment":
assert isinstance(points, dict) and set(points) == {
Expand Down
76 changes: 76 additions & 0 deletions src/datachain/model/utils.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
from collections.abc import Sequence


def validate_img_size(img_size: Sequence[int]) -> Sequence[int]:
"""Validate the image size."""
assert isinstance(img_size, (tuple, list)), "Image size must be a tuple or list."
assert len(img_size) == 2, "Image size must be a tuple or list of 2 integers."
assert all(isinstance(value, int) for value in img_size), (
"Image size must be integers."
)
assert all(value > 0 for value in img_size), "Image size must be positive integers."
return img_size


def validate_bbox(coords: Sequence[float]) -> Sequence[float]:
"""Validate the bounding box coordinates."""
assert isinstance(coords, (tuple, list)), "Bounding box must be a tuple or list."
assert len(coords) == 4, "Bounding box must be a tuple or list of 4 coordinates."
assert all(isinstance(value, (int, float)) for value in coords), (
"Bounding box coordinates must be floats or integers."
)
assert all(value >= 0 for value in coords), (
"Bounding box coordinates must be positive."
)
return coords


def validate_bbox_normalized(
coords: Sequence[float], img_size: Sequence[int]
) -> Sequence[float]:
"""Validate the bounding box coordinates and normalize them to the image size."""
assert isinstance(coords, (tuple, list)), "Bounding box must be a tuple or list."
assert len(coords) == 4, "Bounding box must be a tuple or list of 4 coordinates."
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), (
Copy link
Member

Choose a reason for hiding this comment

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

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)?

"Bounding box normalized coordinates must be floats between 0 and 1."
)

width, height = validate_img_size(img_size)

return [
coords[0] * width,
coords[1] * height,
coords[2] * width,
coords[3] * height,
]


def normalize_coords(
coords: Sequence[int],
img_size: Sequence[int],
) -> list[float]:
"""Normalize the bounding box coordinates to the image size."""
assert isinstance(coords, (tuple, list)), "Coords must be a tuple or list."
assert len(coords) == 4, "Coords must be a tuple or list of 4 coordinates."
assert all(isinstance(value, int) for value in coords), (
"Coords must be a tuple or list of 4 ints."
)

width, height = validate_img_size(img_size)

assert (
0 <= coords[0] <= width
Copy link
Member

Choose a reason for hiding this comment

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

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

and 0 <= coords[1] <= height
and 0 <= coords[2] <= width
and 0 <= coords[3] <= height
), "Bounding box coordinates are out of image size"

return [
coords[0] / width,
coords[1] / height,
coords[2] / width,
coords[3] / height,
]
Loading
Loading