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 support for getting labels from vector tiles #532

Merged
merged 12 commits into from Nov 15, 2018

Conversation

Projects
None yet
2 participants
@lewfish
Copy link
Contributor

lewfish commented Oct 25, 2018

Overview

This PR adds support for using Mapbox vector tiles as a source of labels. See MBTilesVectorSource.

It also adds a new VectorSource abstraction (from which MBTilesVectorSource and GeoJSONVectorSource are derived) and refactors the LabelSources and RasterSources to make use of VectorSources instead of raw URIs. This is all done in a backwards compatible way and deprecation warning are emitted when using outdated key names. Related name changes:

  • ChipClassificationGeoJSONSource -> ChipClassificationLabelSource
  • ObjectDetectionGeoJSONSource -> ObjectDetectionLabelSource
  • SemanticSegmentationRasterSource -> SemanticSegmentationLabelSource
  • raster_source.GeoJSONSource -> raster_source.RasterizedSource

Also, in order to handle vector tiles (and GeoJSON files in the wild), which don't necessarily have class_ids, a flexible class inference mechanism is added. It uses Mapbox GL filters to infer class ids, and this functionality was adapted from label-maker.

Example of building a MBTilesVectorSource:

uri = 'http://foo.com/{z}/{x}/{y}.mvt'
class_id_to_filter = {1: ['has', 'building']}

# default_class_id=None indicates to remove any features that aren't captured by the filters
b = MBTilesVectorSourceConfigBuilder() \
    .with_class_inference(class_id_to_filter=class_id_to_filter,
                                        default_class_id=None) \
    .with_uri(uri) \
    .with_zoom(14) \
    .build()

TODO

  • Add Vegas Spacenet example showing use of OSM vector tiles for labels.
  • Update readthedocs
  • Update changelog

Notes

  • Improving performance is left to issue #557
  • Open questions:
    • Why didn't it work over Potsdam?
    • Why do we get warning about "polygon begins with an inner ring"?
    • Why are there lots of empty geometry collections in OSM?

Testing

Vegas test

See azavea/raster-vision-examples#18 which has an example of using vector tiles on the Vegas dataset. I ran this for all tasks and got it to work.

62

Unit test

The unit test for MBTilesVectorSource uses an extent over Las Vegas that spans two tiles and filters out buildings. The two black squares show the tile boundaries, and the blue rectangle shows the extent of the VectorSource. As expected, all buildings are present, they are cropped against the extent, and the features representing a building that spans the two tiles has been merged into a single building.

las-vegas-vector-tiles

Closes #534
Closes #510
Closes #454

@lewfish lewfish added the review label Oct 25, 2018

@lewfish lewfish force-pushed the lf/vector-tiles branch 5 times, most recently from 18b2b9e to 46ec4a5 Oct 26, 2018

@codecov

This comment has been minimized.

Copy link

codecov bot commented Oct 29, 2018

Codecov Report

Merging #532 into develop will increase coverage by 1.11%.
The diff coverage is 82.74%.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop     #532      +/-   ##
===========================================
+ Coverage    67.74%   68.85%   +1.11%     
===========================================
  Files          158      167       +9     
  Lines         7192     7436     +244     
===========================================
+ Hits          4872     5120     +248     
+ Misses        2320     2316       -4
Impacted Files Coverage Δ
rastervision/data/raster_source/default.py 86.95% <ø> (+0.28%) ⬆️
rastervision/task/semantic_segmentation_config.py 93.33% <ø> (ø) ⬆️
rastervision/data/label_source/api.py 100% <ø> (ø) ⬆️
rastervision/augmentor/augmentor_config.py 83.33% <0%> (ø) ⬆️
rastervision/data/raster_source/api.py 100% <100%> (ø) ⬆️
...vision/data/vector_source/mbtiles_vector_source.py 100% <100%> (ø)
rastervision/runner/experiment_runner.py 39.84% <100%> (ø) ⬆️
rastervision/rv_config.py 88.88% <100%> (ø) ⬆️
rastervision/core/config.py 100% <100%> (ø) ⬆️
rastervision/runner/command_definition.py 92.1% <100%> (ø) ⬆️
... and 78 more

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 5dfff3c...1833301. Read the comment docs.

@lewfish lewfish force-pushed the lf/vector-tiles branch 2 times, most recently from 6d0ff6a to 3a5815a Oct 29, 2018

@lewfish lewfish force-pushed the lf/vector-tiles branch 2 times, most recently from fbdb45c to 9150041 Nov 6, 2018

@lewfish lewfish changed the title WIP: Add VectorSource abstraction and support vector tiles Add support for getting labels from vector tiles Nov 6, 2018

@lewfish lewfish changed the title Add support for getting labels from vector tiles WIP: Add support for getting labels from vector tiles Nov 7, 2018

@lewfish lewfish force-pushed the lf/vector-tiles branch from 9150041 to 0df85e8 Nov 8, 2018

@lewfish lewfish changed the title WIP: Add support for getting labels from vector tiles Add support for getting labels from vector tiles Nov 8, 2018

@lewfish lewfish requested a review from lossyrob Nov 8, 2018

@lewfish lewfish force-pushed the lf/vector-tiles branch from 821ee71 to ee51112 Nov 8, 2018

lewfish added some commits Oct 25, 2018

Install tippecanoe and supermercado
Needed for dealing with vector tiles
Copy label-maker filter code
We tried including label_maker as a dependency, but its dependencies were clashing with
ours and I didn't see how to use pip install --no-deps with our pip setup. And we just need
one file.
Improve backward compatibility
This makes it so that old protobufs still work, so that old predict packages still work.

@lewfish lewfish force-pushed the lf/vector-tiles branch from ee51112 to 1833301 Nov 9, 2018

@lewfish lewfish requested review from jamesmcclain and removed request for lossyrob Nov 13, 2018

@jamesmcclain
Copy link
Member

jamesmcclain left a comment

If maintaining backward compatibility with old prediction packages is an explicit goal, is there a possibility of adding a suitable old prediction package to the integration tests?

@lewfish

This comment has been minimized.

Copy link
Contributor Author

lewfish commented Nov 13, 2018

If maintaining backward compatibility with old prediction packages is an explicit goal, is there a possibility of adding a suitable old prediction package to the integration tests?

Seems like a good idea. Let me see what I can do.

@jamesmcclain
Copy link
Member

jamesmcclain left a comment

I reran the tests, and now there is a different test failure (f1 scores not close enough in an integration test before, failure to build docker image now).

@lewfish

This comment has been minimized.

Copy link
Contributor Author

lewfish commented Nov 15, 2018

I reran the CI job and it passes. Also, I'm working on adding a backward compatibility test. It's more time-consuming that I expected, so I'd like to merge this PR now and add the test in a separate PR.

@jamesmcclain

This comment has been minimized.

Copy link
Member

jamesmcclain commented Nov 15, 2018

Sounds good to me.

@lewfish lewfish merged commit 28b2850 into develop Nov 15, 2018

1 of 2 checks passed

continuous-integration/travis-ci/pr The Travis CI build could not complete due to an error
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details

@lewfish lewfish deleted the lf/vector-tiles branch Nov 15, 2018

@lewfish lewfish removed the review label Nov 15, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.