-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
Vectorized interface for bboxes and keypoints #1577
Conversation
Need to compare with #1396 |
Torchvision augmentations support boxes, while imgaug is not supported. I suspect people would be more interested in how |
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.
Hey @Dipet - I've reviewed your changes and they look great!
General suggestions:
- Ensure the new vectorized interface is well-documented, including examples of how to use it.
- Consider backward compatibility with the previous interface to ease the transition for existing users.
- Perform thorough testing with real-world datasets to ensure the enhancements do not introduce any regressions.
Here's what I looked at during the review
- 🟡 General issues: 4 issues found
- 🟢 Security: all looks good
- 🟡 Testing: 4 issues found
- 🟡 Complexity: 1 issue found
- 🟢 Docstrings: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment to tell me if it was helpful.
@property | ||
def values_dim(self) -> int: | ||
return DATA_DIM | ||
|
||
@property | ||
def internal_type(self) -> Optional[Type[DataWithLabels]]: | ||
return None |
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.
suggestion (code_clarification): Consider explaining the purpose of returning None
for internal_type
in DataProcessor
.
Updated bench result in images/sec Bbboxes
Keypoints
|
Addressed in #1906 |
Simple benchmarks shows significant speedup from 4 to 6 times.
Benchmark:
Results:
Result of bbox and keypoint benchmark from current PR