Skip to content
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 instance segmentation model #3

Open
wants to merge 3 commits into
base: vcap-v0.2
Choose a base branch
from

Conversation

apockill
Copy link
Member

I'm creating this pull request to add an instance segmentation model to our capsule repertoire.

Let's get the code review out of the way, but I don't want to merge it until the experience with BrainFrame is better.

@apockill
Copy link
Member Author

Things to do to improve the experience with BrainFrame:

  1. The database currently has a limit of 256 characters for the coords. We might want to increase this, or have the capsule decrease the resolution of the polygon approximation.
  2. If you add a person tracker, the client crashes. The logic in the client for interpolating tracks is pretty simple right now, and can't interpolate between differing-length instances.

@BryceBeagle BryceBeagle changed the title Added initial files Add instance segmentation model Sep 21, 2020

### Sources
Trained using the [Tensorflow Object Detection API](
https://github.com/tensorflow/models/blob/master/research/object_detection)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: use consistent link syntax

)

@staticmethod
def segm_postprocess(box, raw_cls_mask, im_h, im_w):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Type annotation

return im_mask

@staticmethod
def expand_box(box, scale):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Type annotation

**common_detector_options,
"keep_aspect_ratio": BoolOption(
default=False,
description="If True, it will keep the aspect ratio of "
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: I'm not sure of the technical term for this, but don't use "it will". The "it" can lead to confusion.

ex:
"If True, it will keep the aspect ratio" -> "If True, the aspect ratio will be kept"

@@ -0,0 +1,13 @@
Copyright 2020 Dilili Labs, Inc.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought we were using Aotu in the License files from now on?

@@ -0,0 +1,201 @@
Apache License
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

seems that this file is a duplicate, just with a lowercase name

@@ -0,0 +1,2 @@
[about]
api_compatibility_version = 0.2
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: newline at end of file


## Model
### Architecture & Performance
This model is is a Mask R-CNN with ResNet50 backbone.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is is

this [python demo][python demo].

### Model File Origin
The pretrained model was downloaded from the Open
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"Open OpenCV Model Zoo"

was this what you wanted it to say?

@BryceBeagle
Copy link
Member

The database currently has a limit of 256 characters for the coords.

Postgres has a native Polygon type. We may want to use it:
https://www.postgresql.org/docs/8.3/datatype-geometric.html

@@ -0,0 +1,13 @@
Copyright 2020 Dilili Labs, Inc.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

File duplicate, but with lowercase name?

state: BaseStreamState) -> DETECTION_NODE_TYPE:
frame_h, frame_w, _ = frame.shape

if options["keep_aspect_ratio"]:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you think people will want to turn this off? In other capsules, we always keep the same aspect ratio, right?

@@ -0,0 +1,201 @@
Apache License
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We're not consistent about this, but the license should include the copyright owner as well, which for this model is "Copyright (c) 2019 Intel Corporation".

@velovix
Copy link
Member

velovix commented Sep 21, 2020

Postgres is so cool! BF-1440

)
else:
zipped = zip(boxes, classes, prediction['raw_masks'], scores)
for box, cls, raw_mask, score in zipped:
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Change cls to class_id to match the pattern above

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants