Skip to content
This repository has been archived by the owner on May 7, 2020. It is now read-only.

Added a new dataset: GTA 5 #15

Open
wants to merge 9 commits into
base: master
Choose a base branch
from

Conversation

david-vazquez
Copy link
Contributor

Hi I have added the dataset loader for GTA 5.

Copy link
Owner

@fvisin fvisin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the PR and sorry for the long time it took me to review it!!

There are a few minor modifications to make, but overall looks good to me. Once you are done with them it can be merged. Thank you!

@@ -33,3 +33,6 @@ shared_path = /data/lisatmp4/romerosa/datasets/davis/

[gatech]
shared_path = /data/lisatmp4/dejoieti/data/GATECH/

[gta5]
shared_path = /data/lisatmp4/vazquezd/Datasets/gta5/
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add the dataset to https://github.com/fvisin/dataset_loaders/blob/master/dataset_loaders/__init__.py as well (both the import and in __all__)

class GTA5Dataset(ThreadedDataset):
'''The GTA5 semantic segmentation dataset

The GTA5 dataset [1] consists of 24966 densely labelled frames split into
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Replace [1] with [GTA5]_ please

10 parts for convenience. The class labels are compatible with the CamVid
and CityScapes datasets.

The dataset should be downloaded from [1]_ into the `shared_path`
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[1]_ --> [GTA5]_


References
----------
.. [1] https://download.visinf.tu-darmstadt.de/data/from_games/
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[1] --> [GTA5]

mean = [0, 0, 0]
std = [1, 1, 1]

mapping_type = 'cityscapes'
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't like to have this hardcoded. If having both mappings makes sense please create three classes: a GTA5_abstract class with the common code and two subclasses GTA5_camvid and GTA5_cityscapes that define the relevant attributes.

Unfortunately non_void_nclasses is an attribute of the class and not of the object, so it cannot simply be set in the constructor. It should not be too convoluted to have two classes I hope.

import scipy.io
split = scipy.io.loadmat(os.path.join(self.path, 'split.mat'))
split = split[self.which_set + "Ids"]
for id in split:
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

id is a built-in function in python. Please replace it with something else (e.g., f or name)

for id in split:
filenames.append(str(id[0]).zfill(5)+'.png')
self._filenames = filenames
# print(filenames)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove the commented command

# cycle through the different videos
for prefix in prefix_list:
per_subset_names[prefix] = [el for el in filenames if
el.startswith(prefix)]
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Filtering the filenames by the prefix every time get_names is called seems wasteful: why don't you make self.filenames a dictionary and modify the filenames function to store a list of filenames for each prefix?

This should work but please check it:

def filenames(self):
    if self._filenames is None:
        filenames = {}
        split = scipy.io.loadmat(os.path.join(self.path, 'split.mat'))
        split = split[self.which_set + "Ids"]
        for name in split:
            prefix = name[:6]
            filenames.setdefault(prefix, []).append(name[0].zfill(5) + '.png')
        self._filenames = filenames
    return self._filenames

You can then remove self.prefix_list and the related function, and return self.filenames directly in get_names().

"""
from skimage import io
from PIL import Image
import numpy as np
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Move all the imports at the top of the file

img = io.imread(os.path.join(self.image_path, frame))
img = img.astype(floatX) / 255.

# mask = io.imread(os.path.join(self.mask_path, frame))
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove the commented out line please

@pep8speaks
Copy link

pep8speaks commented Sep 18, 2017

Hello @david-vazquez! Thanks for updating the PR.

Line 298:80: E501 line too long (81 > 79 characters)
Line 300:80: E501 line too long (81 > 79 characters)
Line 305:80: E501 line too long (80 > 79 characters)
Line 888:9: E722 do not use bare except'

Comment last updated on October 23, 2017 at 21:36 Hours UTC

@fvisin
Copy link
Owner

fvisin commented Sep 25, 2017

Thank you again for investigating the issue with the zoom!
Once #18 is merged please rebase this PR so that the parts that don't have to do with GTA get removed from this PR.

@fvisin
Copy link
Owner

fvisin commented Dec 29, 2017

Hey @david-vazquez any update on this? :)

@marcociccone
Copy link
Contributor

@david-vazquez do you have time to work on this? I have a student that could help you closing the PR

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

Successfully merging this pull request may close these issues.

5 participants