-
Notifications
You must be signed in to change notification settings - Fork 1.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
add data benchmark for tsn #57
add data benchmark for tsn #57
Conversation
Codecov Report
@@ 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
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
configs/recognition/tsn/README.md
Outdated
| x | short-side 320 | RandomResizedCrop | 25x3 frames | 70.91 | 89.51 | x | x | x | | ||
|
||
Notes: | ||
|
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.
Suggestion:
- put this paragraph between the section title and the table, and remove the
Notes:
- what does the
se
in the config name represents? very cryptic - for the 320p data, how about in addition running a setting without the first resize augmentation (e.g. https://github.com/open-mmlab/mmaction2/pull/57/files#diff-addd50e00eeedda0c1f02c685ac94a1eR32), since scaling to height 256 defeats the purpose of being "short-side". You may use the lazy augmentation (as in https://github.com/open-mmlab/mmaction2/blob/master/configs/recognition/i3d/i3d_r50_fast_32x2x1_100e_kinetics400_rgb.py#L41). The lazy op would keep as much info as possible.
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.
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.
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.
Okay, so the meaning of scale is not (w, h), but some kind of scale bounds
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.
What does se
mean
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.
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?
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.
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.
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.
btw in the past, we used 320p to denote short-side 320, eg https://github.com/open-mmlab/mmaction2/blob/master/configs/recognition/tsn/tsn_r50_320p_1x1x3_110e_kinetics400_flow.py
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.
Thats OK. I think maybe I can use 320p and 340x256 directly in the config name.
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.
that would be nice
Two more comments:
|
|
Detailed data benchmark for TSN with two data formats, two training augmentations, and two testing protocols (8 combinations).