Add ADE20K dataset #429

mitmul · 2017-09-27T06:06:27Z

This PR adds a semantic segmentation dataset "ADE20K" from MIT Scene Parsing Benchmark http://sceneparsing.csail.mit.edu/

Hakuyume · 2017-09-27T12:14:30Z

chainercv/datasets/ade20k/ade20k_semantic_segmentation_dataset.py

+    p = Pool(2)
+    data_root = download.get_dataset_directory(root)
+    urls = [trainval_url, test_url]
+    ret = [p.apply_async(utils.cached_download, args=(url,)) for url in urls]


cached_download shows an progress report. Is it safe to call multiple cached_download at the same time?

Right. So, actually, it tries to show two progress reports at the same line, then the display of the progress report is broken. Should I do those download sequentially?

I prefer sequential downloading. Is the downloading too slow?

Hakuyume · 2017-09-27T12:15:56Z

tests/datasets_tests/ade20k_tests/test_ade20k.py

+        self.dataset = ADE20KSemanticSegmentationDataset(split=self.split)
+
+    @attr.slow
+    def test_camvid_dataset(self):


test_camvid_dataset?

Hakuyume · 2017-09-27T12:19:21Z

chainercv/datasets/ade20k/ade20k_semantic_segmentation_dataset.py

+            automatically downloaded into
+            :obj:`$CHAINER_DATASET_ROOT/pfnet/chainercv/ade20k`.
+        split ({'train', 'val', 'test'}): Select from dataset splits used in
+            Cityscapes dataset.


Cityscapes?

Oops, I forgot to fix the name... Thanks for pointing out it!

Hakuyume · 2017-09-27T12:22:47Z

chainercv/datasets/ade20k/ade20k_semantic_segmentation_dataset.py

+            When :obj:`split` is either :obj:`train` or :obj:`val`, it returns
+            a tuple consited of a color image and a label whose shapes are
+            (3, H, W) and (H, W), respectively, while :obj:`split` is
+            :obj:`test`, it returns only the color image. H and W are height


If split == 'test', this class is not a semantic segmentation dataset. This behaviour is confusing because other semantic segmentation datasets always return pairs of image and label. This is against the definition of ***SemanticSegmentationDataset.

So, what should I do? Are you suggesting that I should remove the test split from this class or I should treat this class as a dataset that is not a SemanticSegmentationDataset?

Or making the test split as a separate one that is just an ImageDataset could be a possible choice. What do you think?

Among those two solutions, I prefer later one, separating into ADE20KSegmentationDataset and ADE20K(Test)ImageDataset.

OK, I'll try that option. Thanks for the advice!

Among those two solutions, I prefer later one, separating into ADE20KSegmentationDataset and ADE20K(Test)ImageDataset.

Sorry. ADE20KSegmentationDataset -> ADE20KSemanticSegmentationDataset

Hakuyume · 2017-09-27T12:37:02Z

Does ADE20K support other types of annotation? Can we add another ADE20K dataset (e.g. ADE20KInstanceSegmentationDataset)?

mitmul · 2017-09-27T13:10:37Z

Does ADE20K support other types of annotation?

Yes, it does. It has instance-aware segmentation mask and parts segmentation mask.

yuyu2172 · 2017-09-28T00:04:13Z

chainercv/datasets/ade20k/ade20k_utils.py

+# The values used here are copied from CSAILVision/sceneparsing:
+# https://github.com/CSAILVision/sceneparsing
+
+ade20k_label_names = [


How about changing this to a tuple?

yuyu2172 · 2017-09-28T00:04:30Z

chainercv/datasets/ade20k/ade20k_utils.py

+    'flag'
+]
+
+ade20k_label_colors = [


yuyu2172 · 2017-09-28T00:05:27Z

tests/datasets_tests/ade20k_tests/test_ade20k.py

+            assert_is_semantic_segmentation_dataset(
+                self.dataset, len(ade20k_label_names), n_example=10)
+        else:
+            for img in self.dataset[:10]:


How about testing on randomly selected 10 indices.

yuyu2172 · 2017-09-28T00:07:27Z

chainercv/datasets/ade20k/ade20k_semantic_segmentation_dataset.py

+
+def get_ade20k():
+    data_root = download.get_dataset_directory(root)
+    cache_fn = utils.cached_download(url)


cache_fn -> cache_path

yuyu2172 · 2017-09-28T00:08:14Z

chainercv/datasets/ade20k/ade20k_test_image_dataset.py

+
+def get_ade20k():
+    data_root = download.get_dataset_directory(root)
+    cache_fn = utils.cached_download(url)


yuyu2172 · 2017-09-28T00:08:26Z

chainercv/datasets/ade20k/ade20k_test_image_dataset.py

+def get_ade20k():
+    data_root = download.get_dataset_directory(root)
+    cache_fn = utils.cached_download(url)
+    utils.extractall(cache_fn, data_root, os.path.splitext(url)[1])


yuyu2172 · 2017-09-28T00:08:32Z

chainercv/datasets/ade20k/ade20k_semantic_segmentation_dataset.py

+def get_ade20k():
+    data_root = download.get_dataset_directory(root)
+    cache_fn = utils.cached_download(url)
+    utils.extractall(cache_fn, data_root, os.path.splitext(url)[1])


mitmul · 2017-09-28T02:27:59Z

@yuyu2172 Thanks for the comments, I fixed them

yuyu2172 · 2017-09-28T03:09:02Z

tests/datasets_tests/ade20k_tests/test_ade20k.py

+            assert_is_semantic_segmentation_dataset(
+                self.dataset, len(ade20k_label_names), n_example=10)
+        else:
+            idx = np.random.permutation(np.arange(len(self.dataset)))


Sorry, one more thing.
Can you change idx to indices.

Other than that, LGTM.

Hmm. Travis failed with a build issue...

Changed idx to indices

Travis needs to be fixed.

yuyu2172 · 2017-09-28T08:46:15Z

Can you merge master?
Travis should work now thanks to #431

mitmul · 2017-09-28T09:20:31Z

Merged with master.

yuyu2172 · 2017-09-29T03:01:43Z

LGTM

@Hakuyume
What do you think?

Hakuyume · 2017-09-29T03:58:06Z

tests/datasets_tests/ade20k_tests/test_ade20k.py

+    {'split': 'val'},
+    {'split': 'test'}
+)
+class TestADE20KDataset(unittest.TestCase):


How about separating into two classes?

OK, will fix

Hakuyume · 2017-09-29T03:58:24Z

chainercv/datasets/__init__.py

+from chainercv.datasets.ade20k.ade20k_semantic_segmentation_dataset import ADE20KSemanticSegmentationDataset  # NOQA
+from chainercv.datasets.ade20k.ade20k_test_image_dataset import ADE20KTestImageDataset  # NOQA
+from chainercv.datasets.ade20k.ade20k_utils import ade20k_label_colors  # NOQA
+from chainercv.datasets.ade20k.ade20k_utils import ade20k_label_names  # NOQA


According to our naming convention, ade20k_label_colors should be ade20k_semantic_segmentation_label_colors.

https://github.com/chainer/chainercv/blob/master/chainercv/datasets/cityscapes/cityscapes_utils.py#L53

You think this is a problem also?
label_names should be changed as well because it can be different from the one used by Instance Segmentation.

Yes, that is also a problem. Thank you for pointing out.

To summarize:

ade_label_colors --> ade_semantic_segmentation_label_colors

ade_label_names --> ade_semantic_segmentation_label_names

cityscapes_label_colors --> cityscapes_semantic_segmentation_label_colors

cityscapes_label_names --> cityscapes_semantic_segmentation_label_names

Objects for CamVid need not be changed because this dataset only contains semantic segmentation data.

Thank you for summarizing.

Objects for CamVid need not be changed because this dataset only contains semantic segmentation data.

I agree with you.

mitmul · 2017-09-29T08:43:40Z

I resolved all points in your reviews

yuyu2172 · 2017-09-29T21:59:39Z

Thank you.
Oh. Travis is failing.

mitmul · 2017-09-30T04:49:10Z

Fixed the travis errors

Hakuyume · 2017-10-05T02:43:26Z

docs/source/reference/datasets.rst

+.. autoclass:: ADE20KSemanticSegmentationDataset
+
+ADE20KTestImageDataset
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~


The length of ~.

Hakuyume · 2017-10-05T02:44:55Z

chainercv/datasets/ade20k/ade20k_test_image_dataset.py

+url = 'http://data.csail.mit.edu/places/ADEchallenge/release_test.zip'
+
+
+def get_ade20k():


How about moving this function under ade20k_utils?

mitmul · 2017-10-05T06:11:09Z

@Hakuyume Thanks. Reflected your comments.

Hakuyume

LGTM

mitmul added 3 commits September 27, 2017 15:05

Add ADE20K dataset

30c3893

Fix test

30e8496

flake8

f4d8f71

mitmul added the feature label Sep 27, 2017

Hakuyume reviewed Sep 27, 2017

View reviewed changes

mitmul added 2 commits September 27, 2017 21:30

Fix a typo

1ecb49f

fix test

07ccec5

mitmul added 3 commits September 27, 2017 22:04

Separate test dataset

8c676b9

update tests

91585af

update doc

7f913ee

fix test

70826ad

yuyu2172 reviewed Sep 28, 2017

View reviewed changes

Fix some

d0a4d8d

yuyu2172 previously approved these changes Sep 28, 2017

View reviewed changes

yuyu2172 reviewed Sep 28, 2017

View reviewed changes

Fix test

9f70102

Merge branch 'master' of github.com:chainer/chainercv into add-ade20k

7797bd5

Hakuyume reviewed Sep 29, 2017

View reviewed changes

follow reviews

f9b719c

yuyu2172 mentioned this pull request Sep 29, 2017

Change variable names for Cityscapes to be specific to semantic segmentation #437

Merged

Fix test

5b5af14

yuyu2172 assigned Hakuyume Sep 30, 2017

yuyu2172 added this to the v0.7 milestone Oct 5, 2017

Hakuyume reviewed Oct 5, 2017

View reviewed changes

Reflect reviews

1394f6f

mitmul added 2 commits October 5, 2017 15:12

flake8

5c69e06

import os

d6bc134

Hakuyume approved these changes Oct 6, 2017

View reviewed changes

Hakuyume merged commit fb2b5b5 into chainer:master Oct 6, 2017

mitmul deleted the add-ade20k branch October 6, 2017 11:24

		url = 'http://data.csail.mit.edu/places/ADEchallenge/release_test.zip'


		def get_ade20k():

Add ADE20K dataset #429

Add ADE20K dataset #429

Conversation

mitmul commented Sep 27, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Hakuyume commented Sep 27, 2017 • edited Loading

mitmul commented Sep 27, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mitmul commented Sep 28, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yuyu2172 commented Sep 28, 2017 • edited Loading

mitmul commented Sep 28, 2017

yuyu2172 commented Sep 29, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mitmul commented Sep 29, 2017

yuyu2172 commented Sep 29, 2017

mitmul commented Sep 30, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mitmul commented Oct 5, 2017

Hakuyume left a comment

Choose a reason for hiding this comment

mitmul commented Sep 27, 2017 •

edited

Loading

Hakuyume commented Sep 27, 2017 •

edited

Loading

yuyu2172 commented Sep 28, 2017 •

edited

Loading