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

[Feature] ACSampler #381

Closed
wants to merge 3 commits into from
Closed

[Feature] ACSampler #381

wants to merge 3 commits into from

Conversation

SuX97
Copy link
Collaborator

@SuX97 SuX97 commented Nov 25, 2020

This PR adds ACSampler from the paper: SCSampler, including:

  • AC(action classification) samplers based on I-frame, Motion vector, audio.
  • I3D result improvement by using all three samplers.

This PR is challenging, as it is the first model that has a complicated pipeline operated on four kinds of modalities.

ACSampler is the 'simplified' version introduced in the paper, that trains a sampler offline and can be adapted to all kinds of 3D models. As for 'SCSampler'(which requires joint training the recognizer along with the sampler), it will be addressed after ACSampler is re-implemented.

@SuX97 SuX97 marked this pull request as draft November 25, 2020 13:48
@codecov
Copy link

codecov bot commented Nov 25, 2020

Codecov Report

Merging #381 (301988f) into master (f9eb562) will decrease coverage by 0.34%.
The diff coverage is 27.06%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #381      +/-   ##
==========================================
- Coverage   84.65%   84.31%   -0.35%     
==========================================
  Files         121      102      -19     
  Lines        8507     7127    -1380     
  Branches     1394     1157     -237     
==========================================
- Hits         7202     6009    -1193     
+ Misses        952      898      -54     
+ Partials      353      220     -133     
Flag Coverage Δ
unittests 84.29% <27.06%> (-0.35%) ⬇️

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

Impacted Files Coverage Δ
mmaction/models/recognizers/audio_recognizer.py 62.50% <ø> (ø)
mmaction/datasets/pipelines/loading.py 77.44% <5.55%> (-12.65%) ⬇️
mmaction/datasets/audio_visual_dataset.py 16.66% <16.66%> (-60.08%) ⬇️
...maction/models/recognizers/recognizer3d_sampler.py 24.32% <24.32%> (ø)
mmaction/models/samplers/ac_sampler.py 26.47% <26.47%> (ø)
mmaction/models/builder.py 65.51% <66.66%> (+10.25%) ⬆️
mmaction/datasets/pipelines/augmentations.py 94.77% <88.23%> (+0.32%) ⬆️
mmaction/datasets/__init__.py 100.00% <100.00%> (ø)
mmaction/datasets/pipelines/formating.py 94.69% <100.00%> (+4.95%) ⬆️
mmaction/models/__init__.py 100.00% <100.00%> (ø)
... and 71 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 f9eb562...1c2416d. Read the comment docs.

frame_dir = line_split[idx]
if self.audio_prefix is not None:
audio_path = osp.join(self.audio_prefix,
frame_dir) + '.npy'
Copy link
Collaborator

Choose a reason for hiding this comment

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

audio_path = osp.join(self.audio_prefix, frame_dir + '.npy')

@dreamerlin dreamerlin mentioned this pull request Dec 1, 2020
14 tasks
frame_dir = line_split[idx]
if self.audio_prefix is not None:
audio_path = osp.join(self.audio_prefix,
frame_dir) + '.npy'
Copy link
Collaborator

Choose a reason for hiding this comment

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

audio_path = osp.join(self.audio_prefix, frame_dir+'.npy')

frame_dir) + '.npy'
if self.video_prefix:
video_path = osp.join(self.video_prefix,
frame_dir) + '.mp4'
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same

img, (new_w, new_h), interpolation=self.interpolation)
for img in results['imgs']
]
import pdb
Copy link
Collaborator

Choose a reason for hiding this comment

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

remove it

Copy link
Collaborator

Choose a reason for hiding this comment

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

OK, the PR is still in progress

# import pdb
# pdb.set_trace()
test_crop = self.test_cfg.get('test_crop', 3)
test_clip = self.test_cfg.get('test_clip', 30)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can this two var be inferred by batch_size ?

assert (0 <= self.k_prime < self.top_k) and isinstance(
self.k_prime, int)
else:
raise NotImplementedError
Copy link
Collaborator

Choose a reason for hiding this comment

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

combine_type = combination.get('type', None)
assert combine_type in ('convex', 'av_union_list')
if combine_type == ...:
   ...
elif combine_type == ...:
   ...

@dreamerlin dreamerlin mentioned this pull request Dec 2, 2020
15 tasks
@dreamerlin dreamerlin mentioned this pull request Jan 11, 2021
6 tasks
xusu added 3 commits January 13, 2021 17:01
Minor.

Minor.

To squash.

Minor.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants