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 data benchmark for tsn #57

Merged
merged 7 commits into from
Aug 4, 2020

Conversation

kennymckormick
Copy link
Member

Detailed data benchmark for TSN with two data formats, two training augmentations, and two testing protocols (8 combinations).

@codecov
Copy link

codecov bot commented Jul 26, 2020

Codecov Report

Merging #57 into master will increase coverage by 0.45%.
The diff coverage is 91.97%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #57      +/-   ##
==========================================
+ Coverage   84.87%   85.32%   +0.45%     
==========================================
  Files          73       75       +2     
  Lines        3827     4083     +256     
  Branches      618      652      +34     
==========================================
+ Hits         3248     3484     +236     
- Misses        480      493      +13     
- Partials       99      106       +7     
Flag Coverage Δ
#unittests 85.32% <91.97%> (+0.45%) ⬆️

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

Impacted Files Coverage Δ
mmaction/datasets/pipelines/__init__.py 100.00% <ø> (ø)
mmaction/models/__init__.py 100.00% <ø> (ø)
mmaction/models/losses/ohem_hinge_loss.py 75.75% <75.75%> (ø)
mmaction/models/backbones/resnet3d.py 89.69% <76.47%> (-0.89%) ⬇️
mmaction/apis/inference.py 83.01% <77.77%> (-3.83%) ⬇️
mmaction/datasets/pipelines/formating.py 93.85% <85.71%> (+0.34%) ⬆️
mmaction/datasets/rawframe_dataset.py 87.36% <95.23%> (+2.00%) ⬆️
mmaction/datasets/pipelines/loading.py 93.79% <96.62%> (+0.70%) ⬆️
mmaction/models/losses/ssn_loss.py 98.14% <98.14%> (ø)
mmaction/models/backbones/resnet3d_slowfast.py 87.97% <100.00%> (+0.07%) ⬆️
... and 5 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 7dc99cf...cb420fb. Read the comment docs.

| x | short-side 320 | RandomResizedCrop | 25x3 frames | 70.91 | 89.51 | x | x | x |

Notes:

Copy link
Contributor

Choose a reason for hiding this comment

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

Suggestion:

Copy link
Member Author

Choose a reason for hiding this comment

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

Hi, innerlee. This [line] (https://github.com/open-mmlab/mmaction2/pull/57/files#diff-addd50e00eeedda0c1f02c685ac94a1eR32) actually indicates scaling shortedge to 256, so the meaning is correct. The lazy option is False by default, and I will run some experiments with lazy=True, to see if higher resolution makes any difference.

Copy link
Contributor

Choose a reason for hiding this comment

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

Okay, so the meaning of scale is not (w, h), but some kind of scale bounds

Copy link
Contributor

Choose a reason for hiding this comment

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

What does se mean

Copy link
Member Author

Choose a reason for hiding this comment

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

In the latest commit, I add the line: ' In config names, fix(fix_ratio) and se(shortedge) denotes 340x256 data and short-edge 320px data respectively.' to explain the meaning of fix and se. Is it OK to use them in config names to keep them short and concise?

Copy link
Contributor

Choose a reason for hiding this comment

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

long config names are okay. it's better to be able to guess the meaning of each part. so, shortedge is better than se, and fixratio is better than fix.

Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Member Author

Choose a reason for hiding this comment

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

Thats OK. I think maybe I can use 320p and 340x256 directly in the config name.

Copy link
Contributor

Choose a reason for hiding this comment

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

that would be nice

@innerlee
Copy link
Contributor

innerlee commented Aug 3, 2020

Two more comments:

  • what does the _train suffix mean? i guess our configs defaults to training configs
  • could you provide the logs and ckpts? maybe @dreamerlin could help with the uploading

@kennymckormick
Copy link
Member Author

Two more comments:

  • what does the _train suffix mean? i guess our configs defaults to training configs
  • could you provide the logs and ckpts? maybe @dreamerlin could help with the uploading
  1. The config names is further updated to align with the default format @dreamerlin.
  2. The logs and ckpts have been shared to @dreamerlin, I will add links once ready.

@innerlee innerlee merged commit 282a7e5 into open-mmlab:master Aug 4, 2020
@kennymckormick kennymckormick deleted the hd/tsn_data_benchmark branch September 18, 2020 03:00
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

2 participants