Add pose estimation keypoint preprocessing to Sapiens2ImageProcessor - #47199
Add pose estimation keypoint preprocessing to Sapiens2ImageProcessor#47199Sainava wants to merge 5 commits into
Conversation
guarin
left a comment
There was a problem hiding this comment.
Hey thanks a lot for the PR! This looks already pretty good, all the important pieces are here. Left some comments regarding naming, structure, and tests :)
| center = centers[person_idx] | ||
| scale = scales[person_idx] | ||
|
|
||
| hm_coords = (raw_coords - center + 0.5 * scale) / scale * heatmap_size |
There was a problem hiding this comment.
| hm_coords = (raw_coords - center + 0.5 * scale) / scale * heatmap_size | |
| hm_coords = ((raw_coords - center) / scale + 0.5) * heatmap_size |
| person_kps = image_keypoints[person_idx] | ||
|
|
There was a problem hiding this comment.
| person_kps = image_keypoints[person_idx] | |
| person_keypoints = image_keypoints[person_idx] | |
| weights_list.append(torch.zeros((0, heatmap_h, heatmap_w), dtype=torch.float32)) | ||
| continue | ||
|
|
||
| person_kps_tensor = torch.tensor(person_kps, dtype=torch.float32) |
There was a problem hiding this comment.
We prefer to not use abbreviations
| person_kps_tensor = torch.tensor(person_kps, dtype=torch.float32) | |
| person_keypoints_tensor = torch.tensor(person_kps, dtype=torch.float32) |
Same for other variables below
| center = centers[person_idx] | ||
| scale = scales[person_idx] | ||
|
|
||
| hm_coords = (raw_coords - center + 0.5 * scale) / scale * heatmap_size |
There was a problem hiding this comment.
| hm_coords = (raw_coords - center + 0.5 * scale) / scale * heatmap_size | |
| heatmap_coords = (raw_coords - center + 0.5 * scale) / scale * heatmap_size |
| heatmap_h = self.size["height"] // 4 | ||
| heatmap_w = self.size["width"] // 4 |
There was a problem hiding this comment.
Let's pack the whole heatmap generation logic into a generate_upd_gaussian_heatmaps function. Could you also add some tests to test_processing_sapiens2.py that assert that the generated outputs match outputs from the original implementation in https://github.com/facebookresearch/sapiens2/blob/7e5bae88456ac418ff0e58e74106c9fe192055d4/sapiens/pose/src/datasets/codecs/utils/gaussian_heatmap.py#L155?
Something that seems suspicious is that the original uses a np.maximum that is missing from our implementation (might not be necessary): https://github.com/facebookresearch/sapiens2/blob/7e5bae88456ac418ff0e58e74106c9fe192055d4/sapiens/pose/src/datasets/codecs/utils/gaussian_heatmap.py#L229
|
The docs for this PR live here. All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update. |
|
[For maintainers] Suggested jobs to run (before merge) run-slow: sapiens2 |
CI recapDashboard: View test results in Grafana |
|
Hey @guarin, I've implemented the changes. Regarding the Let me know if anything else is needed :) |
guarin
left a comment
There was a problem hiding this comment.
Hey thanks for the updates! Sorry for the delay, was off for a while. I left some comments, my biggest concern is that the current implementation might not match the original one from sapiens2.
| keypoint_heatmap_downscale_factor: int = 4, | ||
| keypoint_heatmap_sigma: float = 6.0, |
There was a problem hiding this comment.
| keypoint_heatmap_downscale_factor: int = 4, | |
| keypoint_heatmap_sigma: float = 6.0, |
Not needed as they are already in **kwargs. Please add the default values above like this:
class Sapiens2ImageProcessor(BeitImageProcessor):
valid_kwargs = Sapiens2ImageProcessorKwargs
...
keypoint_heatmap_downscale_factor = 4
keypoint_heatmap_sigma = 6.0
| kwargs["keypoint_heatmap_downscale_factor"] = keypoint_heatmap_downscale_factor | ||
| kwargs["keypoint_heatmap_sigma"] = keypoint_heatmap_sigma |
There was a problem hiding this comment.
| kwargs["keypoint_heatmap_downscale_factor"] = keypoint_heatmap_downscale_factor | |
| kwargs["keypoint_heatmap_sigma"] = keypoint_heatmap_sigma |
| input_data_format: ChannelDimension | None, | ||
| return_tensors: str | TensorType | None, |
There was a problem hiding this comment.
Are you sure input_data_format can be None here? It is usually only ChannelDimension in the codebase.
| data["pixel_values"] = self._preprocess(images, **images_kwargs) | ||
|
|
There was a problem hiding this comment.
| data["pixel_values"] = self._preprocess(images, **images_kwargs) | |
| data["pixel_values"] = self._preprocess(images, **images_kwargs, boxes=boxes) | |
| processed_segmentation_maps = self._preprocess( | ||
| images=processed_segmentation_maps, **segmentation_maps_kwargs | ||
| ) | ||
|
|
There was a problem hiding this comment.
| processed_segmentation_maps = self._preprocess( | |
| images=processed_segmentation_maps, **segmentation_maps_kwargs | |
| ) | |
| processed_segmentation_maps = self._preprocess( | |
| images=processed_segmentation_maps, **segmentation_maps_kwargs, boxes=boxes | |
| ) | |
| dist_sq = (grid_x.unsqueeze(0) - xs) ** 2 + (grid_y.unsqueeze(0) - ys) ** 2 | ||
| person_heatmaps = torch.exp(-dist_sq / (2 * (sigma**2))) |
There was a problem hiding this comment.
| dist_sq = (grid_x.unsqueeze(0) - xs) ** 2 + (grid_y.unsqueeze(0) - ys) ** 2 | |
| person_heatmaps = torch.exp(-dist_sq / (2 * (sigma**2))) | |
| distance_squared = (grid_x.unsqueeze(0) - xs) ** 2 + (grid_y.unsqueeze(0) - ys) ** 2 | |
| person_heatmaps = torch.exp(-distance_squared / (2 * (sigma**2))) |
| """Generates UDP Gaussian heatmaps and visibility weights from raw keypoint coordinates.""" | ||
| heatmap_height = output_size[0] // downscale_factor |
There was a problem hiding this comment.
Please document the shape and content of the returned lists
|
|
||
| valid_mask = mask * (~out_of_bounds).float() | ||
| person_heatmaps = person_heatmaps * valid_mask | ||
| person_weights = valid_mask.expand_as(person_heatmaps) |
There was a problem hiding this comment.
Hmm I think this is missing the 3 * sigma bounds from the original code. All weights that are outside of a 3 * sigma radius from the keypoint center must be set to 0: https://github.com/facebookresearch/sapiens2/blob/7e5bae88456ac418ff0e58e74106c9fe192055d4/sapiens/pose/src/datasets/codecs/utils/gaussian_heatmap.py#L204-L206
| weights_list.append( | ||
| torch.zeros((0, heatmap_height, heatmap_width), dtype=torch.float32, device=device) | ||
| ) |
There was a problem hiding this comment.
Is this shape correct? Shouldn't the weights have one entry per keypoint instead of one entry for each heatmap pixel?
| def test_generate_udp_gaussian_heatmaps_parity(self): | ||
| import numpy as np | ||
| import torch | ||
|
|
||
| from transformers.models.sapiens2.image_processing_sapiens2 import ( | ||
| box_xywh_to_cxcywh, | ||
| boxes_to_crop_params, | ||
| generate_udp_gaussian_heatmaps, | ||
| ) |
There was a problem hiding this comment.
I think the current test is not ideal as it basically re-implements the logic from the tested function. Could you instead copy the full original function from sapiens2 and write a test that checks that the output from our function matches the output of the original function?
What does this PR do?
As discussed with @guarin in the previous Sapiens2 loss function PR (#46764), this PR implements the preprocessing data pipeline required for Sapiens2 pose estimation fine-tuning.
Specifically, it updates
Sapiens2ImageProcessor(_preprocess_image_like_inputs) to handle raw keypoint coordinates. It implements:(x, y)image coordinates into scaled(H, W)heatmap targets using inverse affine transformations based on the cropped bounding boxes.label_weightsto(num_kps, 1, 1)to utilize PyTorch broadcasting during loss calculation, avoiding dense(num_kps, H, W)allocations.Testing
I ran the full integration test suite for the Sapiens2 Image Processor locally to verify all shapes, visibility masking, and exceptions function as expected.
Command run:
pytest tests/models/sapiens2/test_image_processing_sapiens2.pyResults:
22 passed, 7 skipped(All passing)Before submitting
Pull Request checks?
to it if that's the case.
Who can review?
Hi @guarin , following up on our chat in #46764! Here is the initial implementation for the Sapiens2 preprocessing pipeline we discussed.