Generate suggestions using max point displacement threshold #1862
Conversation
…. Add to yaml file. Create test for new function . . . will need to edit
WalkthroughThe latest update enhances the SLEAP application by introducing functionality to detect significant point displacement in video frames. This includes new configuration options, methods for analyzing displacements, and tests to ensure reliability, all aimed at improving the user experience and analytical capabilities of the application. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant SLEAP_Config
participant Suggestions
participant TestSuite
User->>SLEAP_Config: Update suggestions.yaml
SLEAP_Config->>Suggestions: Read max point displacement config
User->>Suggestions: Request frame suggestions with displacement threshold
Suggestions->>Suggestions: Process max point displacement
Suggestions->>User: Return list of suggested frames
TestSuite->>Suggestions: Run tests for max point displacement
Suggestions->>TestSuite: Return test results
Poem
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 as PR comments)
Additionally, you can add CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://github.com/avatars/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (3)
- sleap/config/suggestions.yaml (2 hunks)
- sleap/gui/suggestions.py (2 hunks)
- tests/gui/test_suggestions.py (1 hunks)
Additional context used
yamllint
sleap/config/suggestions.yaml
[error] 178-178: trailing spaces
(trailing-spaces)
Ruff
sleap/gui/suggestions.py
297-297: Undefined name
Labels(F821)
318-318: Undefined name
Labels(F821)
Additional comments not posted (3)
sleap/config/suggestions.yaml (1)
179-183: Configuration formax point displacementlooks good.The added configuration for
max point displacementfollows the same structure as other methods.sleap/gui/suggestions.py (1)
295-315: Review new methodmax_point_displacement.The method implementation looks correct and is consistent with the existing methods.
Tools
Ruff
297-297: Undefined name
Labels(F821)
tests/gui/test_suggestions.py (1)
26-38: Review new testtest_max_point_displacement_suggestions.The test implementation looks correct and is consistent with the existing tests.
| @@ -175,6 +175,12 @@ main: | |||
| type: double | |||
| default: 0.1 | |||
| range: 0.1,1.0 | |||
|
|
|||
There was a problem hiding this comment.
Remove trailing spaces.
Trailing spaces are unnecessary and should be removed for clean code.
-
+Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Tools
yamllint
[error] 178-178: trailing spaces
(trailing-spaces)
sleap/gui/suggestions.py
Outdated
| max_point_displacement = cls.max_point_displacement, | ||
| ) |
There was a problem hiding this comment.
Fix the dictionary key assignment.
The assignment should be on a single line for consistency.
- max_point_displacement = cls.max_point_displacement,
+ max_point_displacement=cls.max_point_displacement,Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| max_point_displacement = cls.max_point_displacement, | |
| ) | |
| max_point_displacement=cls.max_point_displacement, |
sleap/gui/suggestions.py
Outdated
| def _max_point_displacement_video( | ||
| cls, video: Video, labels: "Labels", displacement_threshold: float | ||
| ): | ||
| lfs = labels.find(video) | ||
| frames = len(lfs) | ||
|
|
||
| if frames < 2: | ||
| return [] | ||
|
|
||
| displacements = [] | ||
| for i in range(1, frames): | ||
| prev_lf = lfs[i - 1] | ||
| curr_lf = lfs[i] | ||
| prev_points = np.array([inst.points_array for inst in prev_lf.instances_to_show]) | ||
| curr_points = np.array([inst.points_array for inst in curr_lf.instances_to_show]) | ||
|
|
||
| if prev_points.shape != curr_points.shape: | ||
| continue | ||
|
|
||
| displacement = np.linalg.norm(curr_points - prev_points, axis=2).sum() | ||
| displacements.append((displacement, curr_lf.frame_idx)) | ||
|
|
||
| frame_idxs = [ | ||
| frame_idx for displacement, frame_idx in displacements if displacement > displacement_threshold | ||
| ] | ||
|
|
||
| return cls.idx_list_to_frame_list(frame_idxs, video) | ||
|
|
There was a problem hiding this comment.
Fix undefined name Labels.
The Labels class should be imported or properly referenced.
- cls, video: Video, labels: "Labels", displacement_threshold: float
+ cls, video: Video, labels: "Labels", displacement_threshold: float
+ ):
+ from sleap.io.dataset import LabelsCommittable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| def _max_point_displacement_video( | |
| cls, video: Video, labels: "Labels", displacement_threshold: float | |
| ): | |
| lfs = labels.find(video) | |
| frames = len(lfs) | |
| if frames < 2: | |
| return [] | |
| displacements = [] | |
| for i in range(1, frames): | |
| prev_lf = lfs[i - 1] | |
| curr_lf = lfs[i] | |
| prev_points = np.array([inst.points_array for inst in prev_lf.instances_to_show]) | |
| curr_points = np.array([inst.points_array for inst in curr_lf.instances_to_show]) | |
| if prev_points.shape != curr_points.shape: | |
| continue | |
| displacement = np.linalg.norm(curr_points - prev_points, axis=2).sum() | |
| displacements.append((displacement, curr_lf.frame_idx)) | |
| frame_idxs = [ | |
| frame_idx for displacement, frame_idx in displacements if displacement > displacement_threshold | |
| ] | |
| return cls.idx_list_to_frame_list(frame_idxs, video) | |
| def _max_point_displacement_video( | |
| cls, video: Video, labels: "Labels", displacement_threshold: float | |
| ): | |
| from sleap.io.dataset import Labels | |
| lfs = labels.find(video) | |
| frames = len(lfs) | |
| if frames < 2: | |
| return [] | |
| displacements = [] | |
| for i in range(1, frames): | |
| prev_lf = lfs[i - 1] | |
| curr_lf = lfs[i] | |
| prev_points = np.array([inst.points_array for inst in prev_lf.instances_to_show]) | |
| curr_points = np.array([inst.points_array for inst in curr_lf.instances_to_show]) | |
| if prev_points.shape != curr_points.shape: | |
| continue | |
| displacement = np.linalg.norm(curr_points - prev_points, axis=2).sum() | |
| displacements.append((displacement, curr_lf.frame_idx)) | |
| frame_idxs = [ | |
| frame_idx for displacement, frame_idx in displacements if displacement > displacement_threshold | |
| ] | |
| return cls.idx_list_to_frame_list(frame_idxs, video) |
Tools
Ruff
318-318: Undefined name
Labels(F821)
roomrys
changed the title
| @@ -291,6 +292,57 @@ def _velocity_video( | |||
|
|
|||
| return cls.idx_list_to_frame_list(frame_idxs, video) | |||
|
|
|||
| def max_point_displacement( | |||
There was a problem hiding this comment.
In order to access this as a classmethod as you do here:
sleap/sleap/gui/suggestions.py
Line 64 in 2789b61
, make sure to wrap it with:
| def max_point_displacement( | |
| @classmethod | |
| def max_point_displacement( |
sleap/config/suggestions.yaml
Outdated
| @@ -175,6 +175,12 @@ main: | |||
| type: double | |||
| default: 0.1 | |||
| range: 0.1,1.0 | |||
|
|
|||
| "max point displacement": | |||
| - name: per_video | |||
There was a problem hiding this comment.
Perhaps we should give this a more accurate/descriptive name such as:
| - name: per_video | |
| - name: threshold |
sleap/gui/suggestions.py
Outdated
| prev_points = np.array([inst.points_array for inst in prev_lf.instances_to_show]) | ||
| curr_points = np.array([inst.points_array for inst in curr_lf.instances_to_show]) | ||
|
|
||
| if prev_points.shape != curr_points.shape: | ||
| continue | ||
|
|
||
| displacement = np.linalg.norm(curr_points - prev_points, axis=2).sum() |
There was a problem hiding this comment.
This calculation for displacement will not gaurantee that we compare the instances we want to because LabeledFrame.instances_to_show returns Instances in the order that they were added to the LabeledFrame - i.e. there is no gaurantee that prev_lf.instances_to_show[0] is annotating the same animal as curr_lf.instances_to_show[0].
Instead, we should be finding Instances that are assigned to the same Track - which will guarantee that they are annotating the same animal (or at least attempting to annotate the same animal).
The Labels.numpy method returns us a numpy array for all LabeledFrames and all Instances sorted by Track:
Lines 2442 to 2468 in 14c21b4
sleap/gui/suggestions.py
Outdated
| # ONCE labels.numpy works: delete lfs ~322 - 328 | ||
| lfs = labels.find(video) | ||
| frames = len(lfs) | ||
|
|
||
| if frames < 2: | ||
| return [] | ||
|
|
||
|
|
||
| video_instances = labels.numpy(video=video, all_frames=True, untracked=False) | ||
| frames = len(video_instances) | ||
|
|
||
| if frames < 2: | ||
| return [] | ||
|
|
||
| # ONCE labels.numpy works: delete print statements ~336 - 340 | ||
| print('type of video_instances: ', type(video_instances)) | ||
| print(video_instances[0]) | ||
| print('type of video_instances[0]: ', type(video_instances[0])) | ||
| print(f"Number of elements returned by labels.numpy(): {video_instances.shape}") | ||
| print(f"Number of elements returned by labels.numpy(): {len(video_instances)}") | ||
|
|
||
|
|
||
| print('type of video_instances: ', type(video_instances)) | ||
| print('type of video_instances[0]: ', type(video_instances[0])) | ||
|
|
||
|
|
||
| displacements = [] | ||
| for idx in range(1, frames): | ||
| prev_points = video_instances[idx-1] | ||
| curr_points = video_instances[idx] | ||
|
|
||
|
|
||
| if prev_points.shape != curr_points.shape: | ||
| continue | ||
|
|
||
| # Mask to identify non-nan values | ||
| valid_mask = ~np.isnan(prev_points) & ~np.isnan(curr_points) | ||
| # Filter out nan values | ||
| valid_prev_points = prev_points[valid_mask].reshape(-1, 2) | ||
| valid_curr_points = curr_points[valid_mask].reshape(-1, 2) | ||
|
|
||
| if valid_prev_points.size == 0 or valid_curr_points.size == 0: | ||
| continue | ||
|
|
||
| displacement = np.linalg.norm(valid_curr_points - valid_prev_points, axis=1).sum() | ||
| displacements.append((displacement, idx)) | ||
|
|
||
| frame_idxs = [ | ||
| frame_idx for displacement, frame_idx in displacements if displacement > displacement_threshold | ||
| ] |
There was a problem hiding this comment.
The suggested approach handles nan values as we want by:
- resulting in nan in the euclidean norm
- Being excluded in the mean calculation for all points in an
Instance
| # ONCE labels.numpy works: delete lfs ~322 - 328 | |
| lfs = labels.find(video) | |
| frames = len(lfs) | |
| if frames < 2: | |
| return [] | |
| video_instances = labels.numpy(video=video, all_frames=True, untracked=False) | |
| frames = len(video_instances) | |
| if frames < 2: | |
| return [] | |
| # ONCE labels.numpy works: delete print statements ~336 - 340 | |
| print('type of video_instances: ', type(video_instances)) | |
| print(video_instances[0]) | |
| print('type of video_instances[0]: ', type(video_instances[0])) | |
| print(f"Number of elements returned by labels.numpy(): {video_instances.shape}") | |
| print(f"Number of elements returned by labels.numpy(): {len(video_instances)}") | |
| print('type of video_instances: ', type(video_instances)) | |
| print('type of video_instances[0]: ', type(video_instances[0])) | |
| displacements = [] | |
| for idx in range(1, frames): | |
| prev_points = video_instances[idx-1] | |
| curr_points = video_instances[idx] | |
| if prev_points.shape != curr_points.shape: | |
| continue | |
| # Mask to identify non-nan values | |
| valid_mask = ~np.isnan(prev_points) & ~np.isnan(curr_points) | |
| # Filter out nan values | |
| valid_prev_points = prev_points[valid_mask].reshape(-1, 2) | |
| valid_curr_points = curr_points[valid_mask].reshape(-1, 2) | |
| if valid_prev_points.size == 0 or valid_curr_points.size == 0: | |
| continue | |
| displacement = np.linalg.norm(valid_curr_points - valid_prev_points, axis=1).sum() | |
| displacements.append((displacement, idx)) | |
| frame_idxs = [ | |
| frame_idx for displacement, frame_idx in displacements if displacement > displacement_threshold | |
| ] | |
| # Get numpy of shape (frames, tracks, nodes, x, y) | |
| labels_numpy = labels.numpy(video=video, all_frames=True, untracked=False) | |
| # Return empty list if not enough frames | |
| n_frames, n_tracks, n_nodes, _ = labels_numpy.shape | |
| if n_frames < 2: | |
| return [] | |
| # Calculate displacements | |
| diff = labels_numpy[1:] - labels_numpy[:-1] # (frames - 1, tracks, nodes, x, y) | |
| euc_norm = np.linalg.norm(diff, axis=-1) # (frames - 1, tracks, nodes) | |
| mean_euc_norm = np.nanmean(euc_norm, axis=-1) # (frames - 1, tracks) | |
| # Find frames where mean displacement is above threshold | |
| threshold_mask = np.any( | |
| mean_euc_norm > displacement_threshold, axis=-1 | |
| ) # (frames - 1,) | |
| frame_idxs = list(np.argwhere(threshold_mask).flatten()) # [0, len(frames - 1)] |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #1862 +/- ##
===========================================
+ Coverage 73.30% 74.34% +1.03%
===========================================
Files 134 135 +1
Lines 24087 24724 +637
===========================================
+ Hits 17658 18382 +724
+ Misses 6429 6342 -87 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Actionable comments posted: 3
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (3)
- sleap/config/suggestions.yaml (2 hunks)
- sleap/gui/suggestions.py (3 hunks)
- tests/gui/test_suggestions.py (1 hunks)
Additional context used
yamllint
sleap/config/suggestions.yaml
[error] 178-178: trailing spaces
(trailing-spaces)
Ruff
sleap/gui/suggestions.py
299-299: Undefined name
Labels(F821)
320-320: Undefined name
Labels(F821)
Additional comments not posted (6)
sleap/config/suggestions.yaml (2)
6-6: LGTM!The new option "max point displacement" has been correctly added to the options list.
179-184: LGTM!The new configuration option for "max point displacement" has been correctly added with appropriate parameters.
sleap/gui/suggestions.py (3)
64-64: LGTM!The new method
max_point_displacementhas been correctly added to the method functions dictionary.
296-317: LGTM!The new method
max_point_displacementhas been correctly implemented to find frames with maximum point displacement above a threshold.Tools
Ruff
299-299: Undefined name
Labels(F821)
296-297: Wrap the method with@classmethod.In order to access this as a
classmethod, make sure to wrap it with@classmethod.+ @classmethod def max_point_displacement(Likely invalid or redundant comment.
tests/gui/test_suggestions.py (1)
27-39: LGTM!The new test function
test_max_point_displacement_suggestionshas been correctly implemented to validate the output of theVideoFrameSuggestions.suggestmethod when using the "max_point_displacement" method.
|
|
||
| "max point displacement": | ||
| - name: displacement_threshold | ||
| label: Maximum Displacement Threshold | ||
| type: int | ||
| default: 10 | ||
| range: 0,999 |
There was a problem hiding this comment.
Remove trailing spaces.
Trailing spaces are unnecessary and should be removed for clean code.
-
+Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "max point displacement": | |
| - name: displacement_threshold | |
| label: Maximum Displacement Threshold | |
| type: int | |
| default: 10 | |
| range: 0,999 | |
| "max point displacement": | |
| - name: displacement_threshold | |
| label: Maximum Displacement Threshold | |
| type: int | |
| default: 10 | |
| range: 0,999 |
Tools
yamllint
[error] 178-178: trailing spaces
(trailing-spaces)
| @classmethod | ||
| def _max_point_displacement_video( | ||
| cls, video: Video, labels: "Labels", displacement_threshold: float | ||
| ): | ||
| # Get numpy of shape (frames, tracks, nodes, x, y) | ||
| labels_numpy = labels.numpy(video=video, all_frames=True, untracked=False) | ||
|
|
||
| # Return empty list if not enough frames | ||
| n_frames, n_tracks, n_nodes, _ = labels_numpy.shape | ||
|
|
||
| if n_frames < 2: | ||
| return [] | ||
|
|
||
| # Calculate displacements | ||
| diff = labels_numpy[1:] - labels_numpy[:-1] # (frames - 1, tracks, nodes, x, y) | ||
| euc_norm = np.linalg.norm(diff, axis=-1) # (frames - 1, tracks, nodes) | ||
| mean_euc_norm = np.nanmean(euc_norm, axis=-1) # (frames - 1, tracks) | ||
|
|
||
| # Find frames where mean displacement is above threshold | ||
| threshold_mask = np.any( | ||
| mean_euc_norm > displacement_threshold, axis=-1 | ||
| ) # (frames - 1,) | ||
| frame_idxs = list( | ||
| np.argwhere(threshold_mask).flatten() + 1 | ||
| ) # [0, len(frames - 1)] | ||
|
|
||
| return cls.idx_list_to_frame_list(frame_idxs, video) | ||
|
|
There was a problem hiding this comment.
Handle nan values in displacement calculation.
The suggested approach handles nan values as we want by:
- Resulting in
nanin the Euclidean norm. - Being excluded in the mean calculation for all points in an
Instance.
# Get numpy of shape (frames, tracks, nodes, x, y)
labels_numpy = labels.numpy(video=video, all_frames=True, untracked=False)
# Return empty list if not enough frames
n_frames, n_tracks, n_nodes, _ = labels_numpy.shape
if n_frames < 2:
return []
# Calculate displacements
diff = labels_numpy[1:] - labels_numpy[:-1] # (frames - 1, tracks, nodes, x, y)
euc_norm = np.linalg.norm(diff, axis=-1) # (frames - 1, tracks, nodes)
mean_euc_norm = np.nanmean(euc_norm, axis=-1) # (frames - 1, tracks)
# Find frames where mean displacement is above threshold
threshold_mask = np.any(
mean_euc_norm > displacement_threshold, axis=-1
) # (frames - 1,)
frame_idxs = list(
np.argwhere(threshold_mask).flatten() + 1
) # [0, len(frames - 1)]
return cls.idx_list_to_frame_list(frame_idxs, video)Committable suggestion was skipped due to low confidence.
Tools
Ruff
320-320: Undefined name
Labels(F821)
|
|
||
| return suggestions | ||
|
|
||
| @classmethod |
There was a problem hiding this comment.
Fix undefined name Labels.
The Labels class should be imported or properly referenced.
- cls, video: Video, labels: "Labels", displacement_threshold: float
+ cls, video: Video, labels: "Labels", displacement_threshold: float
+ ):
+ from sleap.io.dataset import LabelsCommittable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| @classmethod | |
| @classmethod | |
| def _max_point_displacement_video( | |
| cls, video: Video, labels: "Labels", displacement_threshold: float | |
| ): | |
| from sleap.io.dataset import Labels |
Description
Types of changes
Does this address any currently open issues?
Thank you for contributing to SLEAP!
❤️
Summary by CodeRabbit
Summary by CodeRabbit
New Features
max point displacementto help users detect frames with significant point movement.Tests
max point displacementfeature to ensure its accuracy and reliability.