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

feat(deepen): Add initial lidarseg support #191

Open
wants to merge 29 commits into
base: main
Choose a base branch
from

Conversation

KSeangTan
Copy link

@KSeangTan KSeangTan commented Jan 16, 2025

Description

This PR adds features to support 3d painting (lidar segmantic segmentation) in t4 dataset, mainly, it adds:

  1. Download lidar seg from deepen
    • perception_dataset/deepen/download_annotations.py
  2. convert the dowloaded annotations to t4 dataset in annotations/lidarseg.json and lidarseg/annotation/****.bin
    • perception_dataset/t4_dataset/classes/lidarseg.py
    • perception_dataset/deepen/deepen_to_t4_converter.py
    • perception_dataset/deepen/segmentation/painting3d.py
    • perception_dataset/t4_dataset/classes/category.py
  3. To support lidarseg, it also adds a new attribute index to CategoryTable
    • perception_dataset/t4_dataset/classes/category.py

How to test

test data

  1. Update params in convert_deepen_to_t4_lidarseg_3d_painting_sample.yaml:

    • dataset_corresponding: with correct dataset name and deepen id
  2. Download annotations.json

poetry run python perception_dataset/deepen/download_annotations.py --config config/convert_deepen_to_t4_lidarseg_3d_painting_sample.yaml --output_dir ../lidar_semseg_sample/ --label_type paint3d

Test json data from deepen should be in the following format:

[
        {
            "dataset_id": "dummy_dataset_id",
            "file_id": "0.pcd",
            "label_type": "3d_point",
            "label_id": "none",		# Keep it for consistency with downstream tasks
            "label_category_id": "none",	# Keep it for consistency with downstream tasks
            "total_lidar_points": 173430,
            "sensor_id": "lidar",
            "stage_id": "QA",
            "paint_categories": ["car", "wall", ...],
            "lidarseg_anno_file": "lidar_seg/dummy_dataset__id_0.pcd.bin"
        },
        ...
    ]

test command

  1. Update params in convert_deepen_to_t4_lidarseg_3d_painting_sample.yaml:

    • input_anno_file
  2. Convert annotations.json to t4 dataset

poetry run python perception_dataset/convert.py --config config/convert_deepen_to_t4_lidarseg_3d_painting_sample.yaml

Unit tests

poetry run pytest tests/deepen/test_lidarseg_deepen_to_t4_converter.py
poetry run pytest tests/t4_dataset/test_lidarseg_annotation_files_generator.py
poetry run pytest tests/t4_dataset/classes/test_category.py
poetry run pytest tests/t4_dataset/classes/test_lidarseg.py

@KSeangTan KSeangTan self-assigned this Jan 16, 2025
@KSeangTan KSeangTan requested review from kminoda and ktro2828 January 27, 2025 07:29
@KSeangTan KSeangTan changed the title WIP: feat: Add initial lidarseg support feat: Add initial lidarseg support Jan 27, 2025
@KSeangTan KSeangTan changed the title feat: Add initial lidarseg support feat(deepen): Add initial lidarseg support Jan 27, 2025
Comment on lines 176 to 181
parser.add_argument(
"--label_type",
type=str,
default="labels",
help="Annotation type to request labels, supported values: ['labels', 'paint3d']",
)
Copy link
Contributor

Choose a reason for hiding this comment

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

[NITS] choices makes clear options that we can choose.

Suggested change
parser.add_argument(
"--label_type",
type=str,
default="labels",
help="Annotation type to request labels, supported values: ['labels', 'paint3d']",
)
parser.add_argument(
"--label_type",
type=str,
default="labels",
choices=["labels", "paint3d"],
help="Annotation type to request labels, supported values: ['labels', 'paint3d']",
)

Copy link
Author

Choose a reason for hiding this comment

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

Yeah, it looks better
updated

Comment on lines 26 to 27
paint_categories: List[str] = field(default_factory=list)
lidarseg_anno_file: str = ""
Copy link
Contributor

Choose a reason for hiding this comment

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

If we doesn't allow empty list and string, we shouldn't set their default values.

Suggested change
paint_categories: List[str] = field(default_factory=list)
lidarseg_anno_file: str = ""
paint_categories: List[str]
lidarseg_anno_file: str

Copy link
Author

Choose a reason for hiding this comment

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

Make sense, updated

@@ -326,6 +326,7 @@ The item "description" for the category is not implemented for now.
- "token": [str] -- Unique record identifier.
- "name": [str] -- Category name. The latest format is "class" (e.g. car, truck), but "category.class" format (e.g. vehicle.car) is also supported.
- "description": [str] -- Category description. Empty string `""` for now. **(Not available)**
- "index": [int] -- Category index, this is added to support lidarseg.
Copy link
Contributor

Choose a reason for hiding this comment

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

[Q] Should we also allow None for the case category.json doesn't contain the index field, or set it 0 by default?

Copy link
Author

Choose a reason for hiding this comment

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

Let's user None instead of 0 since 0 means non-painted pointclouds in deepen by default

@kminoda
Copy link
Collaborator

kminoda commented Jan 29, 2025

@KSeangTan Would you also update tools_overview.md and t4_format_3d_detailed.md? For instance, we would need to modify the directory structure in the latter document.

Comment on lines 115 to 120
def convert(self):
"""Convert all scene annotations to T4 dataset format."""
if self._label_info is not None and self._label_info.label_type == LabelType.POINT_3D:
scenes_anno_dict = self._format_point_3d_annotations()
else:
scenes_anno_dict = self._format_annotations()
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do you think this modification has a room of improvement?
I was wondering if we can avoid this increasing complexity of the code with nested if-else among convert(), _format_point_3d_annotations(), and _format_annotations. I thought that the simplest way is to reduce the nest e.g. as

if self._label_info is None:
  ...
elif self._label_info.label_type == LabelType.SEGMENTATION_2D:
  ...
elif self._label_info.label_type == LabelType.POINT_3D:
  ...
else:
  ...

But any reason not doing so?

Copy link
Author

@KSeangTan KSeangTan Jan 31, 2025

Choose a reason for hiding this comment

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

I was thinking to remove the nested if-else level in this part by separating two label types here.
However, you are right, I have reformatted this part by introducing
_format_annotations instead

Comment on lines 223 to 274
scene_anno_dict (Dict[int, List[Dict[str, Any]]]): [description]
frame_index_to_sample_token (Dict[int, str]): [description]
frame_index_to_sample_data_token (Dict[int, str]):

scene_anno_dict:
{
0: [
{
"category_name" (str): category name of object,
"instance_id" (str): instance id of object,
"attribute_names" (List[str]): list of object attributes,
"three_d_bbox": {
"translation": {
"x" (float): x of object location,
"y" (float): y of object location,
"z" (float): z of object location,
0: [
{
"category_name" (str): category name of object,
"instance_id" (str): instance id of object,
"attribute_names" (List[str]): list of object attributes,
"three_d_bbox": {
"translation": {
"x" (float): x of object location,
"y" (float): y of object location,
"z" (float): z of object location,
},
"velocity" (Optional[Dict[str, float]]): {
"x" (float): x of object velocity,
"y" (float): y of object velocity,
"z" (float): z of object velocity,
},
"acceleration" (Optional[Dict[str, float]]): {
"x" (float): x of object acceleration,
"y" (float): y of object acceleration,
"z" (float): z of object acceleration,
},
"size": {
"width" (float): width of object size,
"length" (float): length of object size,
"height" (float): height of object size,
},
"rotation": {
"w" (float): w of object quaternion,
"x" (float): x of object quaternion,
"y" (float): y of object quaternion.
"z" (float): z of object quaternion,
},
},
"two_d_box": [
"x" (float): x of left top corner,
"y" (float): y of left top corner,
"w" (float): width of bbox,
"h" (float): height of bbox,
]
"sensor_id": id of the camera
"num_lidar_pts" (int): the number of lidar points in object,
"num_radar_pts" (int): the number of radar points in object,
},
"velocity" (Optional[Dict[str, float]]): {
"x" (float): x of object velocity,
"y" (float): y of object velocity,
"z" (float): z of object velocity,
},
"acceleration" (Optional[Dict[str, float]]): {
"x" (float): x of object acceleration,
"y" (float): y of object acceleration,
"z" (float): z of object acceleration,
},
"size": {
"width" (float): width of object size,
"length" (float): length of object size,
"height" (float): height of object size,
},
"rotation": {
"w" (float): w of object quaternion,
"x" (float): x of object quaternion,
"y" (float): y of object quaternion.
"z" (float): z of object quaternion,
},
},
"two_d_box": [
"x" (float): x of left top corner,
"y" (float): y of left top corner,
"w" (float): width of bbox,
"h" (float): height of bbox,
]
"sensor_id": id of the camera
"num_lidar_pts" (int): the number of lidar points in object,
"num_radar_pts" (int): the number of radar points in object,
},
...
],
1: []. ...
...
],
1: []. ...
Copy link
Collaborator

Choose a reason for hiding this comment

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

Seems that this is an unnecessary change.

Copy link
Author

Choose a reason for hiding this comment

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

Not sure why, but it's modified by my pre-commit automatically
I have reverted the changes

@KSeangTan
Copy link
Author

@KSeangTan Would you also update tools_overview.md and t4_format_3d_detailed.md? For instance, we would need to modify the directory structure in the latter document.

Yeah, I have updated the docs about lidarseg, would you mind taking a look again? Thanks

@KSeangTan KSeangTan closed this Jan 31, 2025
@KSeangTan KSeangTan reopened this Jan 31, 2025
@KSeangTan
Copy link
Author

I got the following error in the CI:

Error: This request has been automatically failed because it uses a deprecated version of `actions/upload-artifact: v3`. Learn more: https://github.blog/changelog/2024-04-16-deprecation-notice-v3-of-the-artifact-actions/

Do you know any fix? @ktro2828 @kminoda

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants