-
Notifications
You must be signed in to change notification settings - Fork 26
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 YOLOX object detection #24
Conversation
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.
Some minor comments, but LGTM
- Pre-trained weights are from COCO (README) - Remove training outputs TODO - Current example uses YOLOX-Tiny (Nano WIP)
} | ||
|
||
/// YOLOX-X pre-trained weights. | ||
pub enum YoloxX { |
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.
Style-wise I am still unsure about the enum variants due to the PascalCase... Names like YoloxS
and YoloxX
look weird to me, so feel free to leave suggestions if you have any!
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.
Ahh, that's fine to me - it seems super minor.
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.
Wow that's big. Looks good overall. I have minor comments. Please see. I am approving ahead.
} | ||
|
||
/// YOLOX-X pre-trained weights. | ||
pub enum YoloxX { |
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.
Ahh, that's fine to me - it seems super minor.
@laggui ready to merge? |
Actually I think we're waiting for the Burn release so we can use the crates version and announce some things. I'll let @nathanielsimard decide when we merge this. |
- Removed init_with methods - Fixed empty MaxPool2d vec initialization
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.
🎉
Closes issue #21
This PR adds a Burn implementation for the YOLOX object detection models. Implementation and weights are based on the official implementation.
TODO
NMS tensor implementationRequiresargsort
We should compare timings w/ the current naive implementation to see if it is worth it since we don't have the nms kernel to perform the computations even more efficientlyI scratched the NMS tensor implementation. Not required at this time.