-
Notifications
You must be signed in to change notification settings - Fork 7.1k
Add mask rcnn to instance segmentation docs #5949
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
Conversation
generate_weights_table( | ||
module=M.detection, | ||
table_name="instance_segmentation", | ||
metrics=[("box_map", "Box MAP"), ("mask_map", "Mask MAP")], |
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.
IMO mean average precision is short handed as mAP and not MAP? That's what I think pycocotools or other report?
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.
I've seen both used. No strong opinion from me
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.
Thanks @oke-aditya ! Looks great so far
generate_weights_table( | ||
module=M.detection, | ||
table_name="instance_segmentation", | ||
metrics=[("box_map", "Box MAP"), ("mask_map", "Mask MAP")], |
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.
I've seen both used. No strong opinion from me
Rendered docs are here I guess it's good to merge |
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.
Thanks a lot @oke-aditya !
Because there are only one model in the Instance Seg
and Keypoint
sections, I wonder if we shouldn't keep them as subsections of a more general "Object Detection, Instance Segmentation and Person Keypoint Detection " section as suggested in #5933 (comment) ?
docs/source/conf.py
Outdated
""" | ||
|
||
module: corresponding torchvision module | ||
table_name: Name of table generated. | ||
metrics: Metrics from weights enum to be used. | ||
include_patterns: List of patterns to include | ||
exclude_patterns: List of patterns to exclude | ||
|
||
Generates table file and saves it to generated folder. | ||
|
||
""" |
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.
I would suggest to remove this docstring, it doesn't add much additional info to the parameter names. A good description would require a much longer docstring, but I don't feel like it's worth doing this right now.
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.
Removed, and changed the subsection. I hope the new layout is what you asked :)
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.
Thanks @oke-aditya , I didn't do a great job at describing what I had in mind, so I pushed directly instead. Docs look like this now:
Nothing is set in stone, we can revisit if we feel like it:)
Thanks a lot for the PR !
Summary: * Add mask rcnn to instance segmentation * Add docs a use nicolas suggestion * remov param * remov docsstring, edit docs * Add one section level Reviewed By: YosuaMichael Differential Revision: D36204384 fbshipit-source-id: 2a9f2f7d524b725ac18f9c9149319e6f1fea6da5 Co-authored-by: Nicolas Hug <[email protected]>
Fixes as discussed offline and #5933 (comment)