Handle attributes returned from nuclio detector #3917

mikhail-treskin · 2021-11-17T15:55:37Z

Motivation and context

Currently CVAT ignores attributes info coming from nuclio detection function along with boxes. Such functionality is required for pipelined inference of several models e.g. detector->age/gender/pose recognition.

How has this been tested?

Tested with corresponding "..: test" targets from vscode configurations (logs may be attached if needed).
Manually checked that attributes returned from nuclio applied both in task auto annotation case and using AI tools in annotation mode for certain frame.

Checklist

I submit my changes into the develop branch
I have added description of my changes into CHANGELOG file
I have updated the documentation accordingly - not applicable
I have added tests to cover my changes - Added new serveless function
I have linked related issues (read github docs)
I have increased versions of npm packages if it is necessary (cvat-canvas,
cvat-core, cvat-data and cvat-ui)

License

I submit my code changes under the same MIT License that covers the project.
Feel free to contact the maintainers if that's a concern.
I have updated the license header for each file (see an example below)

mikhail-treskin · 2021-11-17T15:59:58Z

@bsekachev @nmanovic could you please comment if cvat-ui version should be bumped in package.json in case of such change?

bsekachev · 2021-11-17T19:03:38Z

@mikhail-treskin

case of such change?

Hi, any case (if the patch changes anything). Here, I think need to update patch version number (npm version patch).

bsekachev · 2021-11-18T06:02:18Z

cvat-ui/src/components/annotation-page/standard-workspace/controls-side-bar/tools-control.tsx

@@ -1037,7 +1037,10 @@ export class ToolsControlComponent extends React.PureComponent<Props, State> {
 frame,
 occluded: false,
 source: 'auto',
- attributes: {},
+ attributes: data.attributes.reduce(function(attrs_map: any, attr: any) {
+ attrs_map[attr.spec_id] = attr.value;


as far as I understand, attr.spec_id is returned by nuclio function.
At the same time spec_id is the CVAT specific thing which additionally differs task by task. It means that nuclio function should not return these ids, only attribute names and matching should be implemented on CVAT side.

Ok, it does make sense. And I guess the same is applicable for the changes in backend?

Yep. Matching better to be implemented on backend (for both methods, when call automatic annotation for one frame and for a whole task)

The only one question which I have so far is how to avoid attribute names and spec ids matching on client side in case of one frame auto annotation.

What I could figure out so far regarding attributes adding, is that attrs format expected to be like {spec_id: value} and checked here. So in runInference method I can't create some temporary attribute basing only on name and value, and then in the backend match label+attr_name coming from frontend into spec_id (at least without deeper code changes).

My current implementation of runInference looks like that (not pushed yet until agreement):

runInference={async (task: any, model: Model, body: object) => { try { this.setState({ mode: 'detection', fetching: true }); const result = await core.lambda.call(task, model, { ...body, frame }) let mapping = {}; task.labels.forEach((label: any) => { mapping[label.name] = {}; label.attributes.forEach((attr: any) => { mapping[label.name][attr.name] = attr.id; }) }); const states = result.map( (data: any): any => new core.classes.ObjectState({ shapeType: data.type, label: task.labels.filter((label: any): boolean => label.name === data.label)[0], points: data.points, objectType: ObjectType.SHAPE, frame, occluded: false, source: 'auto', attributes: data.attributes.reduce(function(attrs_map: any, attr: any) { attrs_map[mapping[data.label][attr.name]] = attr.value; return attrs_map; }, {}), zOrder: curZOrder, }), );

Finally, my questions 😄

Is it ok to keep mapping of attr_name returned from nuclio to spec_id in the FE?

If it's ok to keep mapping in the FE, which approach is more preferable, to build mapping object from the code snippet above on render stage or on inference stage?

@mikhail-treskin , could you please look how it is done for labels? For attributes we need to have mostly the same functionality. A serverless function should declare which attributes it supports (as it is done for labels). Based on the information, it is necessary map attributes per task.

Current patch cannot be applied as is to multiple tasks because spec_id will be different for different tasks.

Please let me know if you want to contribute the functionality. It can be quite complex if it is done in production quality. Also need to have a severless function which supports attributes. It will be an example and can be used for testing. I will move the patch into WIP for now.

@nmanovic, yes the PR is definitely WIP, just forgot to mark it properly.

Could you please have a look on changes in latest commit? as @bsekachev proposed previously i'm handling attributes returned from nuclio by name and then maps them into spec_id basing on db task data. In context of your request to declare supported attributes in function config it's not a finall solution, but I suppose that in general functionality will be almost the same.

could you please look how it is done for labels? For attributes we need to have mostly the same functionality. A serverless function should declare which attributes it supports (as it is done for labels). Based on the information, it is necessary map attributes per task.

Does it presume that we also need to have functionality for attributes mapping in UI like it's done for labels?

Also need to have a severless function which supports attributes

As an example can propose pipeline of few OMZ models like face_detection->emotion_recognition+age_gender_recognition (or only on of _recognition)

In general, we really need this functionality for our production usage of CVAT, so will be really appreciated if if you could assist with PR productisation. Don't want to keep it in the fork and struggle with rebases 😄

@mikhail-treskin , the updated patch looks much better. We are more than happy to accept any contribution from our community. Also I will be happy to understand how you are using CVAT in your company and which features are missing. Probably it can be a meeting. Let me know if you are ready to share the information.

An example with OMZ models will be great. Could you please add one or several of them to the PR as serverless functions? If you have any requests to these models, please ping @snosov1. Probably he can help somehow. He also is responsible for OTE: https://github.com/openvinotoolkit/training_extensions

It will be great to map attributes in UI in the future. But let's consider the feature as an optional recommendation. It should not block the PR. I will merge it without the functionality.

Better to decelerate attributes in yml config for a serverless function. Thus it is easy to understand what the function supports.

If you can extend the tutorial: https://openvinotoolkit.github.io/cvat/docs/manual/advanced/serverless-tutorial/, it will be great. But it is an optional recommendation as well.

@nmanovic could you please have a look on serverless function?
I'm not sure regarding face-detection model, tried to choose between 0206 and 0205, looks like 0206 more accurate but much slower than 0205, not sure what is more preferable in case of automatic annotation scenario.

serverless/common/openvino/model_loader.py

nmanovic · 2022-01-11T13:20:35Z

@mikhail-treskin , is the PR ready for review?

mikhail-treskin · 2022-01-12T13:57:47Z

@mikhail-treskin , is the PR ready for review?

Yep, I guess so. Had the question above regarding omz model

I'm not sure regarding face-detection model, tried to choose between 0206 and 0205...

maybe better to address it to @snosov1?

nmanovic · 2022-01-17T21:01:33Z

serverless/openvino/omz/intel/face-detection-0205/function.yaml

+ # attribute names have to be the same as in annotated task, otherwise values will be ignored
+ spec: |
+ [
+ { "id": 0, "name": "face", "attributes": ["age", "gender", "emotion"]}


@mikhail-treskin , thanks for the great PR. Could you please help us to polish the functionality? Each attribute should have a type and possible values. For example, when CVAT returns an attribute (e.g. GET /api/v1/tasks), it returns something like the structure below:

"labels": [ { "id": 0, "name": "age", "attributes": [ { "id": 0, "name": "age", "input_type": "number", "default_value": "25", "values": [ "0", "150", "1" ] } ] } ],

Do you think it is valuable to have something like that here? Otherwise it is unclear which values each attribute has. Also it will be difficult to much them to a corresponding task. Basically, I should be able to take labels and attributes from the definition as is and create a task with them.

Yes, I agree that possible values should be part of function config.
But what about attribute type, i'm not really sure that it does make sense to store it in function config since attribute types is CVAT specific information. Maybe it make sense to add additional attributes verification on backend side, e.g. if certain attribute has type "Select" or "Radio" only one value can be accepted, and check if "Number" attribute coming from nuclio is really convertible to number. Otherwise exception can be raised or fallback to default attribute value.
What do you think about such an approach?

@mikhail-treskin , spec section inside function.yaml is also CVAT specific. Basically I believe that CVAT labels and attributes definition and serverless labels and attributes definition should correspond to each other. Also in the future it is good to implement matching of attributes by name in UI (as it is implemented for labels). It will be necessary to understand the type of an attribute and possible values. I don't suggest implementing matching in UI for the PR. It is another story.

Proposal:

Let's have the same definition for attributes as CVAT server support (see an example above).

If an attribute is supported by a serverless function, but it isn't supported by a corresponding label in CVAT, it should be ignored.

If attributes are matched by name, it is necessary to see if we can cast it to a corresponding type. For example, any attribute can be converted to text type. At the same time text type cannot be converted to select. An attribute, which is matched by name but cannot be casted, should be ignored.

Thus we have a set of labels with attributes from a task. Also we have a number of labels with attributes from a serverless function. We match labels by name. After that we match attributes by name. After that we match attributes by type. During the process some labels and attributes will be filtered. The final list of labels with reduced number of attributes can be covered by our serverless function.
It is a typical case when a serverless function supports more labels and attributes when it is necessary. All of them should be ignored as it is implemented for labels.

@nmanovic Could you please have a look on logic of attributes filtering in lambda manager in latest commit?

If it looks fine for you I'll implement same logic in FE in cvat-ui/src/components/annotation-page/standard-workspace/controls-side-bar/tools-control.tsx

nmanovic · 2022-01-17T21:06:03Z

@mikhail-treskin , could you please help us to fix eslint and pylint linters warnings for the PR?

I believe the PR is a great improvement. We definitely want to have it inside the develop branch. Could you please resolve mentioned issues and I will merge it.

mikhail-treskin · 2022-01-19T12:37:21Z

@mikhail-treskin , could you please help us to fix eslint and pylint linters warnings for the PR?

I believe the PR is a great improvement. We definitely want to have it inside the develop branch. Could you please resolve mentioned issues and I will merge it.

Yes, sure. Should be fixed in latest commit.

cvat/apps/lambda_manager/views.py

nmanovic · 2022-01-20T08:59:54Z

cvat/apps/lambda_manager/views.py

@@ -138,6 +144,10 @@ def to_dict(self):
 response.update({
 'state': self.state
 })
+ if self.kind is LambdaType.DETECTOR:
+ response.update({
+ 'attributes': self.attributes


@mikhail-treskin , Probably it is slightly better to return attributes together with labels.

In general I agree but I was afraid that response structure changing may broke compatibility between BE and FE in some cases which I'm not aware. If it will not affect any functionality I definitely can change the code to return attributes along with labels.

nmanovic · 2022-01-26T13:37:46Z

@mikhail-treskin , thanks for all your time and contribution. Will you be able to continue improving the PR?

nmanovic · 2022-02-18T10:55:00Z

serverless/openvino/omz/intel/face-detection-0205/model_handler.py

+ emotions_model_xml = os.path.join(emotions_base_dir, "emotions-recognition-retail-0003.xml")
+ emotions_model_bin = os.path.join(emotions_base_dir, "emotions-recognition-retail-0003.bin")
+ self.emotions_model = ModelLoader(emotions_model_xml, emotions_model_bin)
+ self.genders_map = ["female", "male"]


@mikhail-treskin , better to load these data from function.yaml.

nmanovic · 2022-02-18T10:55:13Z

serverless/openvino/omz/intel/face-detection-0205/model_handler.py

+ emotions_model_bin = os.path.join(emotions_base_dir, "emotions-recognition-retail-0003.bin")
+ self.emotions_model = ModelLoader(emotions_model_xml, emotions_model_bin)
+ self.genders_map = ["female", "male"]
+ self.emotions_map = ["neutral", "happy", "sad", "surprise", "anger"]


@mikhail-treskin , better to load these data from function.yaml.

nmanovic

LGTM

mikhail-treskin requested review from bsekachev and nmanovic as code owners November 17, 2021 15:55

bsekachev reviewed Nov 18, 2021

View reviewed changes

nmanovic changed the title ~~Handle attributes returned from nuclio detector~~ [WIP] Handle attributes returned from nuclio detector Dec 1, 2021

mikhail-treskin force-pushed the mt/attrs_in_detector branch from 1c038bb to 5f026f0 Compare December 3, 2021 15:23

mikhail-treskin requested a review from bsekachev December 3, 2021 15:43

mikhail-treskin force-pushed the mt/attrs_in_detector branch from c1756a0 to 5aaf0b5 Compare December 8, 2021 14:23

mikhail-treskin force-pushed the mt/attrs_in_detector branch from 431e6a3 to 041d80b Compare December 21, 2021 06:39

mikhail-treskin commented Dec 21, 2021

View reviewed changes

serverless/common/openvino/model_loader.py Outdated Show resolved Hide resolved

mikhail-treskin force-pushed the mt/attrs_in_detector branch 2 times, most recently from 35dd7e6 to 56a2e46 Compare January 10, 2022 11:57

mikhail-treskin changed the title ~~[WIP] Handle attributes returned from nuclio detector~~ Handle attributes returned from nuclio detector Jan 12, 2022

mikhail-treskin force-pushed the mt/attrs_in_detector branch from 56a2e46 to d94c8c7 Compare January 13, 2022 11:34

nmanovic reviewed Jan 17, 2022

View reviewed changes

mikhail-treskin force-pushed the mt/attrs_in_detector branch from d94c8c7 to 8b0d4a7 Compare January 19, 2022 12:36

mikhail-treskin commented Jan 19, 2022

View reviewed changes

cvat/apps/lambda_manager/views.py Outdated Show resolved Hide resolved

nmanovic reviewed Jan 20, 2022

View reviewed changes

cvat/apps/lambda_manager/views.py Outdated Show resolved Hide resolved

nmanovic reviewed Jan 20, 2022

View reviewed changes

mikhail-treskin force-pushed the mt/attrs_in_detector branch from 0774a4c to b232f10 Compare February 1, 2022 13:39

mikhail-treskin added 4 commits February 16, 2022 11:56

Resolve versions cvat-ui conflict

9d23168

Update changelog and license headers

03010ec

Add filtering of not declared attributes

dea02e6

Handle attributes in frame tagging

fb375d7

mikhail-treskin added 4 commits February 16, 2022 11:57

Add serverless function with attributes handling

c2603ff

Fix labels fetching in renderDetectorBlock

bb809c1

Fix linters issues

9d97256

Add attributes filtering in BE

18d4b19

mikhail-treskin force-pushed the mt/attrs_in_detector branch from b232f10 to 18d4b19 Compare February 16, 2022 09:02

nmanovic reviewed Feb 18, 2022

View reviewed changes

nmanovic approved these changes Feb 18, 2022

View reviewed changes

nmanovic merged commit ad11b58 into cvat-ai:develop Feb 18, 2022

nmanovic mentioned this pull request Mar 4, 2022

Release v2.0.0 #4422

Merged

7 tasks

azhiv mentioned this pull request Mar 24, 2022

Prepare UI for attributes configuration #4506

Closed

8 tasks

bsekachev mentioned this pull request Sep 15, 2022

Show attributes returned from a detector #4898

Closed

2 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Handle attributes returned from nuclio detector #3917

Handle attributes returned from nuclio detector #3917

mikhail-treskin commented Nov 17, 2021 •

edited

Loading

mikhail-treskin commented Nov 17, 2021

bsekachev commented Nov 17, 2021 •

edited

Loading

bsekachev Nov 18, 2021

mikhail-treskin Nov 23, 2021

bsekachev Nov 23, 2021

mikhail-treskin Nov 24, 2021 •

edited

Loading

nmanovic Dec 1, 2021

mikhail-treskin Dec 3, 2021 •

edited

Loading

nmanovic Dec 4, 2021

mikhail-treskin Dec 21, 2021

nmanovic commented Jan 11, 2022

mikhail-treskin commented Jan 12, 2022 •

edited

Loading

nmanovic Jan 17, 2022

mikhail-treskin Jan 19, 2022

nmanovic Jan 20, 2022

mikhail-treskin Jan 31, 2022

nmanovic commented Jan 17, 2022

mikhail-treskin commented Jan 19, 2022

nmanovic Jan 20, 2022

mikhail-treskin Jan 20, 2022 •

edited

Loading

nmanovic commented Jan 26, 2022

nmanovic Feb 18, 2022

nmanovic Feb 18, 2022

nmanovic left a comment

Handle attributes returned from nuclio detector #3917

Handle attributes returned from nuclio detector #3917

Conversation

mikhail-treskin commented Nov 17, 2021 • edited Loading

Motivation and context

How has this been tested?

Checklist

License

mikhail-treskin commented Nov 17, 2021

bsekachev commented Nov 17, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mikhail-treskin Nov 24, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mikhail-treskin Dec 3, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nmanovic commented Jan 11, 2022

mikhail-treskin commented Jan 12, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nmanovic commented Jan 17, 2022

mikhail-treskin commented Jan 19, 2022

Choose a reason for hiding this comment

mikhail-treskin Jan 20, 2022 • edited Loading

Choose a reason for hiding this comment

nmanovic commented Jan 26, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nmanovic left a comment

Choose a reason for hiding this comment

mikhail-treskin commented Nov 17, 2021 •

edited

Loading

bsekachev commented Nov 17, 2021 •

edited

Loading

mikhail-treskin Nov 24, 2021 •

edited

Loading

mikhail-treskin Dec 3, 2021 •

edited

Loading

mikhail-treskin commented Jan 12, 2022 •

edited

Loading

mikhail-treskin Jan 20, 2022 •

edited

Loading