-
Notifications
You must be signed in to change notification settings - Fork 278
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
detectron2: require coco2017-minimal data #2022
Conversation
It seems like detectron2 requires coco2017-minimal data. It gets installed with vision-maskrcnn, but if you want to install requirements only for one model we need the installer scripts for the detectron models to download coco2017-minimal.
@davidberard98 has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
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.
LGTM with minor comment
@@ -55,6 +55,7 @@ def remove_tools_directory(): | |||
|
|||
def install_detectron2(model_name, model_dir): | |||
s3_utils.checkout_s3_data("INPUT_TARBALLS", "coco128.tar.gz", decompress=True) |
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 think we could remove this? I remember detectron2 models only requires coco2017-minimal, not coco128 - I don't remember the details though.
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 grepped for coco128 and couldn't find anything other than instances of checkout_s3_data
, so I think you're right - will do that.
@davidberard98 has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
@davidberard98 merged this pull request in 1e03c23. |
It seems like detectron2 requires coco2017-minimal data. It gets installed with vision-maskrcnn, but if you want to install requirements only for one model we need the installer scripts for the detectron models to download coco2017-minimal.