Add clamping_mode for KeyPoints#9234
Conversation
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/vision/9234
Note: Links to docs will display an error until the docs builds have been completed. This comment was automatically generated by Dr. CI and updates every 15 minutes. |
clamping_mode for KeyPointsclamping_mode for KeyPoints
Callidior
left a comment
There was a problem hiding this comment.
Looks good to me overall. Just a few minor remarks.
| clamping_mode: Default is "auto" which relies on the input keypoint' | ||
| ``clamping_mode`` attribute. | ||
| The clamping is done according to the keypoints' ``canvas_size`` meta-data. | ||
| Read more in :ref:`clamping_mode_tuto` |
There was a problem hiding this comment.
clamping_mode_tuto in the docs currently only covers bounding boxes and would need to be updated as well.
| import torch | ||
|
|
||
| from ._bounding_boxes import BoundingBoxes, BoundingBoxFormat, is_rotated_bounding_format | ||
| from ._bounding_boxes import BoundingBoxes, BoundingBoxFormat, CLAMPING_MODE_TYPE, is_rotated_bounding_format |
There was a problem hiding this comment.
With CLAMPING_MODE_TYPE now being used for both bounding boxes and keypoints alike, it would make sense to move its definition out of _bounding_boxes, e.g., directly to tv_tensors.__init__.
|
|
||
| class SetClampingMode(Transform): | ||
| """Sets the ``clamping_mode`` attribute of the bounding boxes for future transforms. | ||
| """Sets the ``clamping_mode`` attribute of the bounding boxes and keypoints for future transforms. |
There was a problem hiding this comment.
I think it could be useful to allow setting the clamping modes of bounding boxes and keypoints to different values by passing a dictionary.
For example:
SetClampingMode("soft")sets the clamping mode of both bounding boxes and keypoints to"soft".SetClampingMode({tv_tensors.BoundingBoxes: "hard", tv_tensors.KeyPoints: "soft"})sets the clamping mode of bounding boxes to"hard"and that of keypoints to"soft".SetClampingMode({tv_tensors.BoundingBoxes: "hard"})sets the clamping mode of bounding boxes to"hard"and leaves that of keypoints unchanged.
test/test_transforms_v2.py
Outdated
| keypoints = tv_tensors.KeyPoints( | ||
| [[0, 100], [0, 100]], canvas_size=(10, 10), clamping_mode=constructor_clamping_mode | ||
| ) | ||
| expected_clamped_output = ( |
There was a problem hiding this comment.
Looks like this line is redundant and can be removed.
Context
This PR proposes to add a
clamping_modeparameter forKeyPoints. This parameter is similar to what have been added forBoundingBoxes(#9128). We are adding this parameter here to avoid the issue observed in #9223, where clamping modifies the position ofKeyPointsand create a misalignment with the actual locations in the transformed image. This PR will need to be supplemented with another PR to implement a class similar toSanitizeBoundingBoxestransform to remove non validKeyPoints, e.g. outside of the image canvas.Implementation details
In the current proposition, we align the possible values for the
clamping_modeparameters with what is already existing forBoundingBoxes, that is"soft","hard", andNone. We would likeKeyPointsclamping mode to be consistent with existing data structures. We propose the following behaviors for each value:"soft": this is the default value if not specified otherwise. This will not apply any clamping operation;None: This will not apply any clamping operation;"hard": This will apply the current default transformation with each coordinates being bounded between0andcanvas_size - 1.We choose this behavior as we believe that we want the default behavior to avoid creating misalignment with the transformed image. In this case,
KeyPointsare not clamped unless explicitly settingclamping_mode="hard".Illustration of the changes
We illustrate below how those changes impact the code.
With
clamping_mode="hard"With
clamping_mode="soft"orNoneTesting
Please run the following unit tests