pytorch / vision

Datasets, Transforms and Models specific to Computer Vision
https://pytorch.org/vision
BSD 3-Clause "New" or "Revised" License
15.95k stars 6.91k forks source link

Using Kinetics400 from references raises a deprecation warning #5787

Open datumbox opened 2 years ago

datumbox commented 2 years ago

🐛 Describe the bug

Using Kinetics400 from references leads to a deprecation warning. The fastest way to reproduce:

torchrun --nproc_per_node=8 train.py --data-path /datasets01/kinetics/070618/ --train-dir=val_avi-480p --val-dir=val_avi-480p --batch-size=64 --sync-bn --test-only --weights R2Plus1D_18_Weights.DEFAULT --cache-dataset

./vision/torchvision/io/video.py:160: UserWarning: The pts_unit 'pts' gives wrong results and will be removed in a follow-up version. Please use pts_unit 'sec'.
  warnings.warn(

It seems this is due to the following line: https://github.com/pytorch/vision/blob/b7c59a086f4e45b8e5e799084e97cca775977a3b/torchvision/datasets/video_utils.py#L327

Which calls the method with the default parameter for pts_unit="pts" which is deprecated: https://github.com/pytorch/vision/blob/b7c59a086f4e45b8e5e799084e97cca775977a3b/torchvision/io/video.py#L155-L163

If the specific option is deprecated, TorchVision shouldn't be using the feature.

Versions

Latest main branch

cc @pmeier @YosuaMichael @bjuncek

pmeier commented 2 years ago

Plus, The Kinetics400 class is also deprecated:

https://github.com/pytorch/vision/blob/1ac6e8b91b980b052324f77828a5ef4a6715dd66/torchvision/datasets/kinetics.py#L308-L311

Not sure why this is not showing up for you though.

datumbox commented 2 years ago

@pmeier Good point. It does. I just missed this because the other warning is spammed 100s of times at the beginning:

UserWarning: The Kinetics400 class is deprecated since 0.12 and will be removed in 0.14.Please use Kinetics(..., num_classes='400') instead.

So we should does this mean that we need to update the reference scripts not to use Kinetics400 then?

bjuncek commented 2 years ago

So this entails of two things:

Now, since we've had to revert the change because of which we introduced 2nd (see #5489), I'll simply remove the warning for now, and document this in #5720 for further action