-
Notifications
You must be signed in to change notification settings - Fork 89
add Sam2 video test #1436
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
base: master
Are you sure you want to change the base?
add Sam2 video test #1436
Conversation
Summary of ChangesHello @zyd-ustc, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request primarily focuses on enabling and testing video processing capabilities for the Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
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.
Code Review
This pull request primarily focuses on adapting image processing utilities for video frames and adding a test for Sam2VideoModel. The modifications in convert_to_rgb and the import utilities are well-implemented. However, the changes to the resize and normalize functions in image_processing_utils_fast.py introduce performance issues by using loops for batch processing. I have provided suggestions to refactor these functions using batched tensor operations, which will significantly improve efficiency. Additionally, I've noted a minor maintainability improvement for the new test file.
| original_shape = image.shape | ||
| batch_dims = original_shape[:-3] | ||
| num_batch = 1 | ||
| for dim in batch_dims: | ||
| num_batch *= dim | ||
| image_flat = image.view(num_batch, *original_shape[-3:]) | ||
|
|
||
| mean = [float(mean[0]), float(mean[1]), float(mean[2])] | ||
| std = [float(std[0]), float(std[1]), float(std[2])] | ||
| normalize = vision.Normalize( | ||
| mean=mean, | ||
| std=std, | ||
| ) | ||
| images = [] | ||
| for img in image: | ||
| images.append(normalize(img.permute(1, 2, 0).asnumpy())) | ||
| return ms.tensor(images).permute(0, 3, 1, 2) | ||
| normalized_images = [] | ||
| for img in image_flat: | ||
| normalized_img = normalize(img.permute(1, 2, 0).asnumpy()) | ||
| normalized_images.append(ms.tensor(normalized_img).permute(2, 0, 1)) | ||
|
|
||
| normalized_flat = mint.stack(normalized_images, dim=0) | ||
| _, new_C, new_H, new_W = normalized_flat.shape | ||
| return normalized_flat.view(*batch_dims, new_C, new_H, new_W) |
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.
The normalize function iterates over each image in the batch, converting it to a NumPy array for processing. This is inefficient. Normalization is a simple arithmetic operation (image - mean) / std that can be performed directly on the entire batch as a tensor operation using broadcasting. This would be significantly faster and avoid unnecessary data conversions.
| original_shape = image.shape | |
| batch_dims = original_shape[:-3] | |
| num_batch = 1 | |
| for dim in batch_dims: | |
| num_batch *= dim | |
| image_flat = image.view(num_batch, *original_shape[-3:]) | |
| mean = [float(mean[0]), float(mean[1]), float(mean[2])] | |
| std = [float(std[0]), float(std[1]), float(std[2])] | |
| normalize = vision.Normalize( | |
| mean=mean, | |
| std=std, | |
| ) | |
| images = [] | |
| for img in image: | |
| images.append(normalize(img.permute(1, 2, 0).asnumpy())) | |
| return ms.tensor(images).permute(0, 3, 1, 2) | |
| normalized_images = [] | |
| for img in image_flat: | |
| normalized_img = normalize(img.permute(1, 2, 0).asnumpy()) | |
| normalized_images.append(ms.tensor(normalized_img).permute(2, 0, 1)) | |
| normalized_flat = mint.stack(normalized_images, dim=0) | |
| _, new_C, new_H, new_W = normalized_flat.shape | |
| return normalized_flat.view(*batch_dims, new_C, new_H, new_W) | |
| mean = ms.tensor(mean, dtype=image.dtype) | |
| std = ms.tensor(std, dtype=image.dtype) | |
| # Reshape mean and std to broadcast across batch and spatial dimensions | |
| view_shape = (1,) * (image.ndim - 3) + (image.shape[-3], 1, 1) | |
| mean = mean.view(view_shape) | |
| std = std.view(view_shape) | |
| return (image - mean) / std |
tests/transformers_tests/models/sam2_video/test_modeling_sam2_video.py
Outdated
Show resolved
Hide resolved
…video.py Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Updated the normalization function to simplify the process by removing unnecessary flattening and looping. Enhanced the documentation to clarify input shape and output characteristics.
What does this PR do?
Fixes # (issue)
1.modify convert_to_rgb,do_resize,do_normalize for video frames
Adds # (feature)
1.add fast test for sam2_video
2.cv2 error info
Before submitting
What's New. Here are thedocumentation guidelines
Who can review?
Anyone in the community is free to review the PR once the tests have passed. Feel free to tag
members/contributors who may be interested in your PR.
@xxx