-
Notifications
You must be signed in to change notification settings - Fork 3k
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
Prepare UI for attributes configuration #4506
Conversation
@nmanovic @bsekachev Can you please review the approach for automapping label attributes? This is a follow up PR for functionality developed in scope of #3917 |
@azhiv , is it ready? If so, please remove 'draft' mode. Also could you please look at linter issues? Indeed I believe the problem with copyright header: |
@klakhov , could you please look at the PR? |
d2c4395
to
425806a
Compare
@klakhov I just finished the implementation, can you please trigger the approval checks? |
cvat-ui/src/components/annotation-page/standard-workspace/controls-side-bar/tools-control.tsx
Outdated
Show resolved
Hide resolved
cvat-ui/src/components/annotation-page/standard-workspace/controls-side-bar/tools-control.tsx
Show resolved
Hide resolved
@klakhov A big thanks for your feedback! In fact I missed this case during testing, focusing only on "positive" test cases. I'll fix this issue. |
@azhiv Currently we don't test 'AI tools' functionality due to CI reasons, so it's better to check as many cases as possible manually |
425806a
to
0da2d0b
Compare
@klakhov I fixed the issue that you pointed out, and added a small usability enhancement (attributes are automapped during labels selection even if the label names differ). Could you please review the changes? |
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.
Cool, the issue seems to be fixed.
But it seems we have the same problem with attributes. If attribute name is different from model's, even though I mapped them, after annotation is completed my attributes are set to default. I think we should cover this case too.
As you can see on my screenshot, if I'm using label1(with attrs gender, age) everything works fine, and if I'm using label2(with attrs gender123, age123) they are defaulted.
cvat-ui/src/components/annotation-page/standard-workspace/controls-side-bar/tools-control.tsx
Outdated
Show resolved
Hide resolved
cvat-ui/src/components/annotation-page/standard-workspace/controls-side-bar/tools-control.tsx
Show resolved
Hide resolved
@klakhov I also noticed this behaviour, but the reason for that is empty attributes array received from the BE I asked @mikhail-treskin, he needs time to investigate this. |
1bc76ef
to
c5f86af
Compare
@azhiv Alright, let us know if you can figure something out 🙂 |
@klakhov Please help me to understand the requirements. When I run the automatic annotation against the following configuration: The two non-mapped attributes gain default values which is fine, but the first one's value - Adult - is not in the list of allowed ones - [1, 2, 3]. So the question is - what value is |
Check the attributes returned by nuclio function call and reject those that have either incompatible types or values.
The code in lambda_manager didn't account for attributes mappings that had different names thus returning an empty set of attributes because it couldn't find the correct match. Fix this by getting proper mapping from `attrMapping` property of the input data.
42ee591
to
2c876f9
Compare
I believe if serverless response for an attribute value is out of specification, we should just ignore it. Maybe to show a single warning message in case when we run serverless function on one frame. |
@azhiv , I would agree with @bsekachev, if we have an attribute value that we are not expected to have, it shouldnt be written into mapped value. It would be great to see warning message about such situation just to let user see what happened. |
There's a bunch of lambda manager tests that failed as a result of my recent changes. I was able to localize the problem, but I'd like to get sure and rerun the tests locally before I push the corrected code. How can lambda manager tests be executed in my local environment? This one |
@bsekachev @klakhov Thanks for your feedback! |
I have solved it.I found that the respond method of Automatic Annotation is different with AI Tool in jobs page, the attrsMapping is missing from the args, so we can't get it. |
Hi @azhiv Thank you for the contribution. You can see corresponding commit here :) |
@bsekachev Great news, at last! Thanks for sharing. |
Motivation and context
This PR allows attributes mapping that belong to certain labels (model and task) with ability to pre-map attributes based on their names.
The values returned from nuclio function are also processed during inference to filter out attributes with incompatible types/values.
How has this been tested?
Checklist
develop
branchcvat-core, cvat-data and cvat-ui)
License
Feel free to contact the maintainers if that's a concern.