HobbitLong / RepDistiller

[ICLR 2020] Contrastive Representation Distillation (CRD), and benchmark of recent knowledge distillation methods
BSD 2-Clause "Simplified" License
2.11k stars 389 forks source link

Add backward compat properties to CIFAR100 dataset classes #14

Open ml-illustrated opened 4 years ago

ml-illustrated commented 4 years ago

Hello, fan of your great work. In trying to reproduce your CIFAR100 results via README example commands, they would end up in an error, e.g.,

python train_student.py --path_t ./save/models/resnet32x4_vanilla/ckpt_epoch_240.pth --distill crd --model_s resnet8x4 -a 0 -b 0.8 --trial 1
Files already downloaded and verified
Traceback (most recent call last):
  File "train_student.py", line 345, in <module>
    main()
  File "train_student.py", line 157, in main
    mode=opt.mode)
  File "...RepDistiller/dataset/cifar100.py", line 211, in get_cifar100_dataloaders_sample
    percent=percent)
  File "...RepDistiller/dataset/cifar100.py", line 124, in __init__
    num_samples = len(self.train_data)
AttributeError: 'CIFAR100InstanceSample' object has no attribute 'train_data'

A simple change is to add four properties to a new class CIFAR100BackCompat to minimize the code changes, as submitted in this pull request.

For additional background, here's the Torchvision's MNIST source for reference: https://pytorch.org/docs/stable/_modules/torchvision/datasets/mnist.html#MNIST

relevant section:

    @property
    def train_labels(self):
        warnings.warn("train_labels has been renamed targets")
        return self.targets

    @property
    def test_labels(self):
        warnings.warn("test_labels has been renamed targets")
        return self.targets

    @property
    def train_data(self):
        warnings.warn("train_data has been renamed data")
        return self.data

    @property
    def test_data(self):
        warnings.warn("test_data has been renamed data")
        return self.data

Tested the changes with:

torch==1.3.1
torchvision==0.4.2
HobbitLong commented 4 years ago

@ml-illustrated , Thank you for your nice input, I like your modification a lot!

Just wonder would it be possible to make it compatible with both torchvision 0.4.2 and previous torchvision which still uses train_data stuff, e.g. torchvision 0.2.1?

ml-illustrated commented 4 years ago

Hi Yonglong,

Sure, I'll take a look and test the changes against at torchvision 0.2.1 and update the PR once my changes can support both.

Gerald

On Fri, Jan 3, 2020 at 3:42 PM Yonglong Tian notifications@github.com wrote:

@ml-illustrated https://github.com/ml-illustrated , Thank you for your nice input, I like your modification pretty much!

Just wonder would it be possible to make it compatible with both torchvision 0.4.2 and previous torchvision which still uses train_data stuff, e.g. torchvision 0.2.1?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/HobbitLong/RepDistiller/pull/14?email_source=notifications&email_token=AIOT564T7VKMHVXREOUARKLQ37EOTA5CNFSM4KCRDFQ2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEICKVFI#issuecomment-570731157, or unsubscribe https://github.com/notifications/unsubscribe-auth/AIOT566FRUDEUVATR64RPSTQ37EOTANCNFSM4KCRDFQQ .