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

Add Pascal VOC Class Segmentation #37

Closed
wants to merge 3 commits into from
Closed

Conversation

desimone
Copy link

@desimone desimone commented Jan 21, 2017

  • I use the train/val/test split defined the text files found in ./VOCdevkit/VOC2012/ImageSets/Segmentation/*.txt
  • Perhaps it makes sense to rename the class in line with its associated task like ms-coco does with CocoDetection and CocoCaptions. How about PascalSegmentation?
  • Should any special considerations or changes be made for semantic segmentation datasets?


def __getitem__(self, index):
img = Image.open(self.images[index]).convert('RGB')
target = Image.open(self.masks[index]).convert('RGB')

This comment was marked as off-topic.

if self.transform is not None:
print("transform was not none")
img = self.transform(img)
# todo(bdd) : perhaps transformations should be applied differently to masks?

This comment was marked as off-topic.

return

# downloads file
if os.path.isfile(fpath) and \

This comment was marked as off-topic.

self.masks = []
with open(os.path.join(split_f), "r") as lines:
for line in lines:
image = os.path.join(image_dir, line.rstrip('\n') + ".jpg")

This comment was marked as off-topic.

splits_dir = os.path.join(voc_root, 'ImageSets/Segmentation')
split_f = os.path.join(splits_dir, 'train.txt')
if not self.train:
split_f = os.path.join(splits_dir, ' trainval.txt')

This comment was marked as off-topic.

@fmassa
Copy link
Member

fmassa commented Jan 21, 2017

Thanks for the PR!
Overall this looks good. I have a few inline comments, and some more generic remarks:

  • I think we should mention that this is the segmentation dataset. Maybe something like VOCSegmentation?
  • I'd rather let the user select the split himself, instead on only allowing for train and trainval splits.
  • This is specific to VOC 2012, maybe it would be good to let the user select between different year contests?
  • I was planning on adding a VOCDetection dataset as well. Some preliminary version can be found in https://github.com/pytorch/examples/pull/21/files#diff-0344e770fabb635e92d12f8b67f3504a . I'll merge it in when this PR gets merged.

I think we will need to figure out a better way of applying random transforms to both inputs and targets, as discussed in #9 .

@desimone
Copy link
Author

desimone commented Jan 21, 2017

Thanks for the review, @fmassa. I have a few follow up questions while I wait for the other datasets to download...

  • I've renamed the class to VOCSegmentation.
  • Can you elaborate on how you'd want the user to select the split themselves? Should the list be built from all available masks and then shuffled? Sorry, I don't fully track.
  • I could add multiple years without too much trouble. I'm curious, what would the usecase be for using 2007 over say 2012?
  • What would be the most idiomatic way to handle errors. At certain points I use asserts. Not sure how I feel about that.
  • Common operations like archive downloading, extracting, hash checking, file io etc seem common amongst all datasets. Maybe we could abstract that functionality?
  • I'll defer how to uniformly apply transforms to both mask and images per Random transforms for both input and target? #9.

@j-min
Copy link

j-min commented Apr 27, 2018

Any updates on this?

@fmassa
Copy link
Member

fmassa commented Apr 27, 2018

@j-min I'll be revisiting this PR before I release my implementation of Detectron. I'm currently in the cleanup process,

@fmassa
Copy link
Member

fmassa commented Dec 6, 2018

Replaced by #663, thanks a lot for the original PR!

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

Successfully merging this pull request may close these issues.

3 participants