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] Support VIS evaluation for YouTube-VIS task. #501

Merged
merged 36 commits into from
Apr 26, 2022
Merged

[Add] Support VIS evaluation for YouTube-VIS task. #501

merged 36 commits into from
Apr 26, 2022

Conversation

pixeli99
Copy link
Collaborator

There are three main modifications here.

  1. Add mmtrack/core/evaluation/eval_vis.py
  2. When converting the dataset to coco format, add width and height info to the key of video.
  3. Override the function of evaluate in mmtrack/datasets/youtube_vis_dataset.py.

For the first part, the JSON format used for training is converted into the native JSON format of YouTube-VIS for calculation.

@codecov
Copy link

codecov bot commented Apr 18, 2022

Codecov Report

Merging #501 (939940a) into master (55ca8d9) will decrease coverage by 0.29%.
The diff coverage is 67.31%.

@@            Coverage Diff             @@
##           master     #501      +/-   ##
==========================================
- Coverage   73.35%   73.06%   -0.30%     
==========================================
  Files         126      129       +3     
  Lines        7441     8055     +614     
  Branches     1395     1562     +167     
==========================================
+ Hits         5458     5885     +427     
- Misses       1555     1675     +120     
- Partials      428      495      +67     
Flag Coverage Δ
unittests 73.01% <67.31%> (-0.26%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
mmtrack/core/evaluation/ytviseval.py 64.11% <64.11%> (ø)
mmtrack/core/evaluation/ytvis.py 64.93% <64.93%> (ø)
mmtrack/datasets/youtube_vis_dataset.py 77.23% <83.67%> (+54.56%) ⬆️
mmtrack/core/evaluation/eval_vis.py 90.62% <90.62%> (ø)
mmtrack/core/evaluation/__init__.py 100.00% <100.00%> (ø)
mmtrack/datasets/pipelines/processing.py 70.90% <0.00%> (-5.46%) ⬇️
mmtrack/datasets/sot_train_dataset.py 82.82% <0.00%> (-3.04%) ⬇️
mmtrack/datasets/pipelines/transforms.py 87.15% <0.00%> (-2.99%) ⬇️
mmtrack/models/sot/siamrpn.py 79.88% <0.00%> (-1.68%) ⬇️
mmtrack/models/trackers/base_tracker.py 86.25% <0.00%> (+1.52%) ⬆️
... and 2 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 55ca8d9...939940a. Read the comment docs.

@pixeli99
Copy link
Collaborator Author

I need to submit two JSON files for unit testing, but they can't pass Pre-commit. How should I solve this problem?

@JingweiZhang12
Copy link
Collaborator

JingweiZhang12 commented Apr 21, 2022

The overall structure needs to be adjusted, we should try to reuse the current code and only add some necessary functions. Some suggestions are as the following:

  1. Try to only add a file containing the YTVOSeval class, because we only want to add the function of evaluation, rather than dataset loading and paring functions.
  2. To my observation, the function of YTVOS is similar to the CocoVideoDataset and 'YoutubeVISDataset'. Can we only add some necessary functions into the
    YoutubeVISDataset and overload some functions of YoutubeVISDataset?
  3. Is the function get_vis_json necessary? To my observation, its contents of annotations are similar to the tools/convert_datasets/youtubvis/youtubevis2coco.py
  4. Some naming problems. We try not to use uppercase letters for the name of normal variables and try to make it easy to understand what your function does according to the name of the function.

mmtrack/core/evaluation/eval_vis.py Outdated Show resolved Hide resolved
mmtrack/core/evaluation/eval_vis.py Outdated Show resolved Hide resolved
mmtrack/core/evaluation/eval_vis.py Outdated Show resolved Hide resolved
mmtrack/core/evaluation/eval_vis.py Outdated Show resolved Hide resolved
mmtrack/core/evaluation/eval_vis.py Outdated Show resolved Hide resolved
mmtrack/core/evaluation/eval_vis.py Outdated Show resolved Hide resolved
mmtrack/core/evaluation/eval_vis.py Outdated Show resolved Hide resolved
mmtrack/core/evaluation/eval_vis.py Outdated Show resolved Hide resolved
mmtrack/datasets/youtube_vis_dataset.py Outdated Show resolved Hide resolved
mmtrack/datasets/youtube_vis_dataset.py Outdated Show resolved Hide resolved
mmtrack/datasets/youtube_vis_dataset.py Outdated Show resolved Hide resolved
mmtrack/datasets/youtube_vis_dataset.py Outdated Show resolved Hide resolved
mmtrack/datasets/youtube_vis_dataset.py Outdated Show resolved Hide resolved
mmtrack/datasets/youtube_vis_dataset.py Outdated Show resolved Hide resolved
mmtrack/datasets/youtube_vis_dataset.py Outdated Show resolved Hide resolved
mmtrack/datasets/youtube_vis_dataset.py Outdated Show resolved Hide resolved
for idx in instance_info:
cur_video_len = len_video[instance_info[idx][0]['video_id']]
len_videos[img_info['video_id']] = max(1, img_info['frame_id'] + 1)
for ins_id in instance_infos:
Copy link
Collaborator

Choose a reason for hiding this comment

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

The last review has mentioned! Too many instance_infos[ins_id] in this piece of code, we could set instance_infos = instances_infos[ins_id], and use instance_infos rather than instance_infos[ins_id] in the following code

bbox[frame_id[ann_info['image_id']]] = ann_info['bbox']
area[frame_id[ann_info['image_id']]] = ann_info['area']
for ann_info in instance_infos[ins_id]:
segm[frame_id_mapping[
Copy link
Collaborator

Choose a reason for hiding this comment

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

The same problems as the above. Too many repeated use of dictionary indexes this piece of code.
frame_id = frame_id_mapping[ann_info['image_id']], and only use frame_id below.

len_video[img_info['video_id']] = max(1, img_info['frame_id'] + 1)
for idx in instance_info:
cur_video_len = len_video[instance_info[idx][0]['video_id']]
len_videos[img_info['video_id']] = max(1, img_info['frame_id'] + 1)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
len_videos[img_info['video_id']] = max(1, img_info['frame_id'] + 1)
video_id = img_info['video_id']
len_videos[video_id] = len_videos.get(video_id, 0) + 1

instance_info = defaultdict(list)
frame_id = defaultdict(list)
len_video = defaultdict(list)
instance_infos = defaultdict(list)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Deprecate the old code, we could use the coco api to realize this function as the following:

VIS = defaultdict(list)
the 'categories' and 'videos' has already loaded in this dataset, don't need to reload it.
for ins_id, img_ids in self.instancesToImgs.items():
    instance_infos = []
    for img_id in img_ids:
           for ann in self.coco.imgToAnns[img_id]:
                if ann['instance_id'] == instance_id:
                    add some code to load ann information of this instance
    VIS['annotations'].append(instance_infos)

@JingweiZhang12
Copy link
Collaborator

JingweiZhang12 commented Apr 25, 2022

Deprecate the old code of convert_vis_format, we could use the neat coco api to realize this function as the following:

VIS = defaultdict(list)
the 'categories' and 'videos' has already loaded in this dataset, don't need to reload `self.ann_file`.
for ins_id, img_ids in self.coco.instancesToImgs.items():
    instance_infos = []
    for img_id in img_ids:
           for ann in self.coco.imgToAnns[img_id]:
                if ann['instance_id'] == instance_id:
                    add some code to load ann information of this instance
    VIS['annotations'].append(instance_infos)

Please check the above pseudo-code to make it run rightly.

Copy link
Collaborator

@GT9505 GT9505 left a comment

Choose a reason for hiding this comment

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

just for removing the request review tag.

mmtrack/datasets/youtube_vis_dataset.py Outdated Show resolved Hide resolved
mmtrack/datasets/youtube_vis_dataset.py Show resolved Hide resolved
mmtrack/datasets/youtube_vis_dataset.py Show resolved Hide resolved
mmtrack/datasets/youtube_vis_dataset.py Show resolved Hide resolved
mmtrack/core/evaluation/eval_vis.py Outdated Show resolved Hide resolved
mmtrack/core/evaluation/eval_vis.py Outdated Show resolved Hide resolved
mmtrack/core/utils/ytvis.py Outdated Show resolved Hide resolved
mmtrack/core/utils/ytvis.py Outdated Show resolved Hide resolved
tests/test_data/test_datasets/test_vis_dataset.py Outdated Show resolved Hide resolved
tests/test_data/test_datasets/test_vis_dataset.py Outdated Show resolved Hide resolved
@pixeli99 pixeli99 requested a review from GT9505 April 25, 2022 10:34
mmtrack/core/evaluation/eval_vis.py Outdated Show resolved Hide resolved
mmtrack/datasets/youtube_vis_dataset.py Outdated Show resolved Hide resolved
mmtrack/datasets/youtube_vis_dataset.py Outdated Show resolved Hide resolved
mmtrack/datasets/youtube_vis_dataset.py Outdated Show resolved Hide resolved
tests/data/demo_vis_data/ann.json Outdated Show resolved Hide resolved
tests/data/demo_vis_data/results.json Outdated Show resolved Hide resolved
mmtrack/datasets/youtube_vis_dataset.py Outdated Show resolved Hide resolved
@pixeli99 pixeli99 requested a review from GT9505 April 26, 2022 03:49
@GT9505 GT9505 merged commit d231cfc into open-mmlab:master Apr 26, 2022
@pixeli99 pixeli99 deleted the eval_vis branch June 10, 2022 02:55
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.

None yet

3 participants