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

[GSoC2024] Fix import of outside track shapes in Datumaro-based formats #7669

Merged
merged 51 commits into from
Apr 19, 2024

Conversation

Yeek020407
Copy link
Contributor

@Yeek020407 Yeek020407 commented Mar 23, 2024

Motivation and context

Main Issue: #7571
Related Issue: #2339

I've reproduced the issue mentioned in #7571 when exporting and importing annotations using both the Datumaro and Coco 1.0 formats. Specifically, the "Switch outside" attribute isn't being applied as expected. After some investigation, I pinpointed the root cause to be the absence of the "outside" attribute in the exported annotations.

To address this, I've made adjustments to the binding.py file to bypass the track_id during annotation import. This modification appears to solve the issue regarding the "Switch outside" attribute. However, it introduces a new concern: the potential loss of information, including keyframes and track_id.

While this workaround offers a temporary fix, I'm contemplating a more holistic approach that entails properly handling the "outside" attribute during both the export and import processes of annotations. This method could preserve the integrity of the annotations while ensuring the functionality of the "Switch outside" attribute.

I'm reaching out for feedback or suggestions on my proposed solution. Is there a preference for one of these approaches, or might there be another avenue I haven't considered?

Looking forward to your insights.

Checklist

  • I submit my changes into the develop branch
  • I have created a changelog fragment
  • I have updated the documentation accordingly
  • I have added tests to cover my changes
  • I have linked related issues (see 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.

@zhiltsov-max
Copy link
Contributor

zhiltsov-max commented Mar 28, 2024

Hi, thank you for sending the PR! Just disabling tracks in the export doesn't seem to be a proper solution, as the whole point of using the outside property is to use it with tracks. Probably, something along these lines should be adjusted, as this is the common logic for several formats. The handling logic (and so, the solution) should be similar to this fragment.

@Yeek020407
Copy link
Contributor Author

Yeek020407 commented Mar 28, 2024

Hi, thank you for sending the PR! Just disabling tracks in the export doesn't seem to be a proper solution, as the whole point of using the outside property is to use it with tracks. Probably, something along these lines should be adjusted, as this is the common logic for several formats. The handling logic (and so, the solution) should be similar to this fragment.

Thanks for your advice. Will make necessary changes!

@Yeek020407 Yeek020407 requested a review from nmanovic as a code owner April 3, 2024 06:02
@Yeek020407
Copy link
Contributor Author

Hi @zhiltsov-max , I have made necessary changes and add a test to it. Please have a look

@zhiltsov-max zhiltsov-max changed the title [GSoC2024] Fix Proposal for Issue #7571 [GSoC2024] Fix import of outside track shapes in Datumaro-based formats Apr 3, 2024
@Yeek020407 Yeek020407 requested a review from azhavoro as a code owner April 7, 2024 08:18
@Yeek020407
Copy link
Contributor Author

Yeek020407 commented Apr 7, 2024

Screenshot 2024-04-07 at 10 11 39 PM

Hii, @zhiltsov-max, I have transferred the code to tests/python and fixed the bug that arose. I have checked with all the REST API, SDK and CLI tests. Please take a look.

@zhiltsov-max
Copy link
Contributor

Hi, please fix the issues reported by the CI.

@Yeek020407
Copy link
Contributor Author

Hi, please fix the issues reported by the CI.

Hope the changes will fix the issues flagged by the CI.

@Yeek020407 Yeek020407 requested a review from zhiltsov-max April 8, 2024 09:09
Copy link
Contributor

@zhiltsov-max zhiltsov-max left a comment

Choose a reason for hiding this comment

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

Please update the tests, and we can merge this.

@Drise13
Copy link

Drise13 commented Apr 17, 2024

@Drise13, you wrote about some code snippet in #7669 (comment). Could you provide information about how you could obtain it? Specifically, the points field is expected to be empty in skeleton annotations.

By snippet, I meant "subset of the json data file". If the skeleton points are supposed to be empty, why are there both ones with and without points? Additionally, when running an export to CVAT, it tries to interpolate the tracks of the skeleton which has no points and throws a broadcast error. My intuition tells me that skeleton tracks should not try to be interpolated and only use the points. That seems to work correctly when I cleaned out the skeletons with no points. It may be that it fixed it because there were no other keyframes to try to interpolate from, but I'm not certain on that.

As for the purpose of this PR, if this is not the right PR for this investigation, should I open a new issue instead?

@zhiltsov-max
Copy link
Contributor

@Drise13,

By snippet, I meant "subset of the json data file".

What is this file?

Additionally, when running an export to CVAT, it tries to interpolate the tracks of the skeleton which has no points and throws a broadcast error. My intuition tells me that skeleton tracks should not try to be interpolated and only use the points.

Agree, it seems like the source of the problem, according to your findings.

As for the purpose of this PR, if this is not the right PR for this investigation, should I open a new issue instead?

There is the issue which you mentioned before. If it describes the problem you faced, then it should be discussed there.

Copy link
Contributor

@zhiltsov-max zhiltsov-max left a comment

Choose a reason for hiding this comment

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

cvat/apps/dataset_manager/bindings.py Outdated Show resolved Hide resolved
@Yeek020407
Copy link
Contributor Author

@zhiltsov-max Thanks for the suggestion. I have added the new test for this.

@Yeek020407 Yeek020407 requested a review from zhiltsov-max April 18, 2024 08:32
@Yeek020407
Copy link
Contributor Author

Hi @zhiltsov-max , I have added the example you provided into our test and added some checking on the number of outside = True in each elements.

@Yeek020407 Yeek020407 requested a review from zhiltsov-max April 18, 2024 13:38
cvat/apps/dataset_manager/bindings.py Show resolved Hide resolved
cvat/apps/dataset_manager/bindings.py Outdated Show resolved Hide resolved
cvat/apps/dataset_manager/bindings.py Outdated Show resolved Hide resolved
cvat/apps/dataset_manager/bindings.py Outdated Show resolved Hide resolved
@zhiltsov-max zhiltsov-max merged commit c5d8a16 into cvat-ai:develop Apr 19, 2024
32 checks passed
@zhiltsov-max
Copy link
Contributor

Thank you for contributing into the project!

@cvat-bot cvat-bot bot mentioned this pull request Apr 26, 2024
@NicolasPetermann134
Copy link

Hi @zhiltsov-max !
I'm facing the same issue: ValueError: operands could not be broadcast together with shapes (0,) (6,)

I found two annotations in backup where "points":[] for a skeleton track.

I removed them and tried to create from backup but I am facing this error - KeyError: "There is no item named 'task.json' in the archive"

I am 100% that in my zip file there is a task.json tough.
my zip has following structure:
dataset

  • annotations.json
  • task.json
  • data
    • all images

Would highly appreciate some help.
Regards

@zhiltsov-max
Copy link
Contributor

@NicolasPetermann134,

I don't see how it's related to this PR or issues focused.

my zip has following structure:

What is dataset, a directory? If it's a directory, it should not be present in the backup archive.

@NicolasPetermann134
Copy link

@zhiltsov-max
I commented on this case as I saw here #7669 (comment) the idea of deleting the instances with empty points...

dataset is the zipped file.
I have here shared my backups https://1drv.ms/f/s!AiG5XaoBkxRVm9YYwD1G7yHEUrE01g?e=tPhDNN

One is the original backup which is not modified - this one I can use to create a new task without any issues.
For the other one I only modified annotation.json - I deleted there the instances with empty points, I didn't touch the task.json but I get the error KeyError: "There is no item named 'task.json' in the archive"

What could be the issue?

@zhiltsov-max
Copy link
Contributor

@Eldies , please look at the issue.

@NicolasPetermann134
Copy link

hi @Eldies, any news on this?

@NicolasPetermann134
Copy link

ftr: I could solve the problem. I could export the dataset easily without errors when I tried it on the self hosted version.

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.

4 participants