-
Notifications
You must be signed in to change notification settings - Fork 101
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
Add triangulated 3d pose attribute #2082
base: liezl/add-gui-elements-for-sessions
Are you sure you want to change the base?
Add triangulated 3d pose attribute #2082
Conversation
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
sleap/io/cameras.py
Outdated
@@ -837,7 +847,8 @@ def get_cam(self, instance: Instance) -> Optional[Camcorder]: | |||
|
|||
def update_points( | |||
self, | |||
points: np.ndarray, | |||
points_3d: np.ndarray, | |||
instance_groups: List['InstanceGroup'], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We don't need to take in a list of InstanceGroup
s here.
instance_groups: List['InstanceGroup'], |
sleap/io/cameras.py
Outdated
# Check if points are 3D | ||
is_3d = points_3d.shape[-1] == 3 | ||
if not is_3d: | ||
raise ValueError("Expected 3D points with shape (M, T, N, 3).") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is redundant since we have a n_coords == 3 check below.
# Check if points are 3D | |
is_3d = points_3d.shape[-1] == 3 | |
if not is_3d: | |
raise ValueError("Expected 3D points with shape (M, T, N, 3).") |
sleap/io/cameras.py
Outdated
# Check that the correct shape was passed in | ||
n_views, n_instances, n_nodes, n_coords = points_3d.shape | ||
assert n_views == len( | ||
self.cams_to_include | ||
), f"Expected {len(self.cams_to_include)} views, got {n_views}." | ||
assert n_instances == len( | ||
instance_groups | ||
), f"Expected {len(instance_groups)} instances, got {n_instances}." |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We expect the 3D shape to be n_nodes, n_coords
only
# Check that the correct shape was passed in | |
n_views, n_instances, n_nodes, n_coords = points_3d.shape | |
assert n_views == len( | |
self.cams_to_include | |
), f"Expected {len(self.cams_to_include)} views, got {n_views}." | |
assert n_instances == len( | |
instance_groups | |
), f"Expected {len(instance_groups)} instances, got {n_instances}." | |
# Check that the correct shape was passed in | |
n_nodes, n_coords = points_3d.shape |
sleap/io/cameras.py
Outdated
# Reproject 3D points into 2D points for each camera view | ||
pts_reprojected = reproject( | ||
points_3d, | ||
calib=self.session.camera_cluster, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We don't have a InstanceGroup.session
attribute, so we'll need to find another way to get the camera_cluster
-> see InstanceGroup.camera_cluster
sleap/io/cameras.py
Outdated
pts_reprojected = reproject( | ||
points_3d, | ||
calib=self.session.camera_cluster, | ||
excluded_views=self.excluded_views, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Similarly, there is no InstanceGroup.excluded_views
. I think we should either add this as an input into InstanceGroup.update_points
(and get rid of the cams_to_include
input) or derive it from the existing cams_to_include
input and InstanceGroup.camera_cluster
attribute.
sleap/io/cameras.py
Outdated
# Update points for each `InstanceGroup` | ||
for ig_idx, instance_group in enumerate(instance_groups): | ||
# Ensure that `InstanceGroup`s is in this `FrameGroup` | ||
self._raise_if_instance_group_not_in_frame_group( | ||
instance_group=instance_group | ||
) | ||
# Update points for the instance group | ||
instance_group.points = points_reprojected[ig_idx] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We will just be passing in points for a single InstanceGroup
, so we won't be needing this here (but, this will still be similar to how we call InstanceGroup.update_points
from FrameGroup.upsert_points
).
# Update points for each `InstanceGroup` | |
for ig_idx, instance_group in enumerate(instance_groups): | |
# Ensure that `InstanceGroup`s is in this `FrameGroup` | |
self._raise_if_instance_group_not_in_frame_group( | |
instance_group=instance_group | |
) | |
# Update points for the instance group | |
instance_group.points = points_reprojected[ig_idx] |
sleap/io/cameras.py
Outdated
# Check that correct shape was passed in | ||
n_views, n_nodes, _ = points.shape | ||
n_views, n_nodes, _ = points_3d.shape |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
points_3d
will have only n_nodes, 3
shape. But, we can do this check on points_reprojected
# Check that correct shape was passed in | |
n_views, n_nodes, _ = points.shape | |
n_views, n_nodes, _ = points_3d.shape | |
# Check that correct shape was passed in | |
n_views, n_nodes, _ = points_reprojected.shape |
sleap/io/cameras.py
Outdated
@@ -883,13 +943,13 @@ def update_points( | |||
if not isinstance(instance, PredictedInstance): | |||
instance_oks = compute_oks( | |||
gt_points[cam_idx, :, :], | |||
points[cam_idx, :, :], | |||
points_3d[cam_idx, :, :], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here we want to use points_reprojected
points_3d[cam_idx, :, :], | |
points_reprojected[cam_idx, :, :], |
sleap/io/cameras.py
Outdated
) | ||
oks_scores[cam_idx] = instance_oks | ||
|
||
# Update the points for the instance | ||
instance.update_points( | ||
points=points[cam_idx, :, :], exclude_complete=exclude_complete | ||
points=points_3d[cam_idx, :, :], exclude_complete=exclude_complete |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here we want to use points_reprojected
points=points_3d[cam_idx, :, :], exclude_complete=exclude_complete | |
points=points_reprojected[cam_idx, :, :], exclude_complete=exclude_complete |
@@ -2331,6 +2381,7 @@ def upsert_points( | |||
points=instance_points, | |||
cams_to_include=self.cams_to_include, | |||
exclude_complete=exclude_complete, | |||
bounds=bounds, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Make sure this input name matches the name you decided on for projection bounds in the definition for InstanceGroup.update_points
…d updated doc string
n_coords, cams_to_include, excluded_views, len(self.camera_cluster)
Description
Added triangulated 3d pose attribute
Types of changes
Does this address any currently open issues?
[list open issues here]
Outside contributors checklist
Thank you for contributing to SLEAP!
❤️