fix splitting CelebA dataset + corresponding test #4377
Conversation
|
Hi @VassilisCN! Thank you for your pull request and welcome to our community. Action RequiredIn order to merge any pull request (code, docs, etc.), we require contributors to sign our Contributor License Agreement, and we don't seem to have one on file for you. ProcessIn order for us to review and merge your suggested changes, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA. Once the CLA is signed, our tooling will perform checks and validations. Afterwards, the pull request will be tagged with If you have received this in error or have any questions, please contact us at cla@fb.com. Thanks! |
|
Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks! |
There was a problem hiding this comment.
Hey @VassilisCN. Thanks for pointing this out. This is a regression from #3656. I have a few minor comments inline.
Co-authored-by: Philip Meier <github.pmeier@posteo.de>
Co-authored-by: Philip Meier <github.pmeier@posteo.de>
Co-authored-by: Philip Meier <github.pmeier@posteo.de>
Summary: Reviewed By: kazhang Differential Revision: D30898333 fbshipit-source-id: cb9ef043eca01be3f07dea50148e0a1867797575 Co-authored-by: Vassilis Nicodemou <nikodim@ics.forth.gr> Co-authored-by: Philip Meier <github.pmeier@posteo.de>
|
I've not gone through the source codes but I did an experiment in Torchvision V 0.12.0.dev20211118+cu102. It seems to me that the index for choosing the image and the annotation don't match. For annotations, I checked the 'gender'. gender[1] is for image[0], gender[2] is for image[1], ... (image[0] misses a gender annotation). I did the experiments in google cloab using torchvision V 0.12.0.dev20211118+cu102. I went through my own codes and didn't find any bugs. is it the case or should I check again? |
|
@mohammad-brdrn I'm not sure what you are doing. "gender" is not one of the attributes that CelebA provides. Could you post a snippet with unexpected or wrong behavior? |
While working on the CelebA dataset, I noticed that when splitting the dataset (into 'train'/'valid'/'test' or take it as a whole 'all') only the target (the attributes) were coherent with the split. The input images were always the same, the only thing changing was their number. I traced the code and noticed that the mask that was created for picking the correct samples for the split was only applied on the targets (the attributes) and not the input images. Therefore I fixed the above issue by applying the mask on the input images as well. Also, I added a corresponding test function that checks if each individual split ('train', 'valid', 'test') merged together would create the same input images as if taking the whole dataset ('all'). This test Fails without the aforementioned fix.
cc @pmeier