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 tests for Cityscapes Dataset #1079

Merged
merged 4 commits into from
Jul 4, 2019
Merged

Conversation

RJT1990
Copy link
Contributor

@RJT1990 RJT1990 commented Jul 3, 2019

  • Mocked the Cityscapes dataset and the different target types (semantic, instance, polygon, color) and modes (gtFine, gtCoarse).
  • Added a generic_segmentation_dataset_test- checks the outputs (image and mask) are both PIL.Image
  • Small bug fix to Cityscapes dataset - incorrect zip location target_dir_zip for coarse mode (discovered with tests ;))

Tried to base this on the style of the other dataset tests - it's a bit more involved with Cityscapes mocking, but let me know if you have any further ideas/refinements for testing and I can incorporate.

Copy link
Member

@fmassa fmassa left a comment

Choose a reason for hiding this comment

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

This looks awesome, thanks a lot for the great PR!

I'm just waiting for CI to finish, and then this is good to merge!

@TheCodez
Copy link
Contributor

TheCodez commented Jul 3, 2019

Can you add a test for having multiple target types, e.g semantic and polygon?

@codecov-io
Copy link

codecov-io commented Jul 3, 2019

Codecov Report

Merging #1079 into master will increase coverage by 0.59%.
The diff coverage is 0%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1079      +/-   ##
==========================================
+ Coverage   63.92%   64.52%   +0.59%     
==========================================
  Files          68       68              
  Lines        5406     5406              
  Branches      829      829              
==========================================
+ Hits         3456     3488      +32     
+ Misses       1707     1666      -41     
- Partials      243      252       +9
Impacted Files Coverage Δ
torchvision/datasets/cityscapes.py 70% <0%> (+50%) ⬆️
torchvision/datasets/folder.py 81.25% <0%> (-1.25%) ⬇️
torchvision/transforms/functional.py 70.23% <0%> (-1.16%) ⬇️
torchvision/transforms/transforms.py 81.18% <0%> (-0.6%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5508815...d3b48b8. Read the comment docs.

@RJT1990
Copy link
Contributor Author

RJT1990 commented Jul 3, 2019

@TheCodez yep can do +1

@fmassa
Copy link
Member

fmassa commented Jul 3, 2019

@RJT1990 tests have passed, do you want to implement the improvements mentioned by @TheCodez in this PR or in a follow-up PR?

@RJT1990
Copy link
Contributor Author

RJT1990 commented Jul 3, 2019

@fmassa I'll do it in this PR I think +1

@RJT1990
Copy link
Contributor Author

RJT1990 commented Jul 3, 2019

Okay that's added now @TheCodez, @fmassa

Copy link
Contributor

@TheCodez TheCodez left a comment

Choose a reason for hiding this comment

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

LGTM 👍

def cityscapes_root():

def _make_image(file):
PIL.Image.fromarray(np.zeros((1024, 2048, 3), dtype=np.uint8)).save(file)
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't know how big the impact is but maybe we could use a smaller image size here instead of the original size to make the tests run faster?

Copy link
Member

Choose a reason for hiding this comment

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

We are not performing any significant operations on the inputs, so this should not be a problem speed-wise.
But good point!

@fmassa fmassa merged commit 7d8cc19 into pytorch:master Jul 4, 2019
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.

4 participants