search

pysal / tobler

Spatial interpolation, Dasymetric Mapping, & Change of Support
https://pysal.org/tobler
BSD 3-Clause "New" or "Revised" License
145 stars 30 forks source link

[WIP] `STRTree` parallel implementation #112

Closed darribas closed 3 years ago

darribas commented 3 years ago

This takes the single-core implementation in #110 and adds multi-core processing for the spatial index query and the intersection/area calculation steps. Very much work in progress so not to merge yet, but I wanted to get the logic down and start a PR. @martinfleis and are hopefully working on this in early Jan and shipping a fully fledged version soon, it'd be great if we could take this one over the line for the January release of the metapackage? At any rate, this will go as "experimental" (possibly in its own separate function, independent of _area_tables_binning to show it's bleeding edge.

cc' @martinfleis, @knaaptime, @sjsrey

codecov-io commented 3 years ago

Codecov Report

Merging #112 (c9fa024) into master (982390a) will decrease coverage by 5.37%. The diff coverage is 23.94%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #112      +/-   ##
==========================================
- Coverage   83.67%   78.30%   -5.38%     
==========================================
  Files          15       15              
  Lines         680      742      +62     
==========================================
+ Hits          569      581      +12     
- Misses        111      161      +50
Impacted Files Coverage Δ
tobler/area_weighted/__init__.py 100.00% <ø> (ø)
.../area_weighted/_vectorized_raster_interpolation.py 63.88% <0.00%> (ø)
tobler/dasymetric/__init__.py 100.00% <ø> (ø)
tobler/dasymetric/masked_area_interpolate.py 90.90% <0.00%> (ø)
tobler/tests/test_area_join.py 100.00% <ø> (ø)
tobler/tests/test_pysal_integration.py 100.00% <ø> (ø)
tobler/util/__init__.py 100.00% <ø> (ø)
tobler/area_weighted/area_interpolate.py 67.76% <15.78%> (-16.11%) :arrow_down:
tobler/model/glm.py 90.00% <50.00%> (ø)
tobler/tests/test_interpolators.py 95.45% <66.66%> (-4.55%) :arrow_down:
... and 4 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 982390a...c9fa024. Read the comment docs.

knaaptime commented 3 years ago

does this supersede #110? or should we merge that while this is wip?

martinfleis commented 3 years ago

@knaaptime this builds on top of #110. We can merge that while this is in WIP.

sjsrey commented 3 years ago

@knaaptime this builds on top of #110. We can merge that while this is in WIP.

110 has been merged.

darribas commented 3 years ago

OK this implementation is ready and a test is added. this may fail as the parallel implementation requires master versions of pygeos and geopandas.

knaaptime commented 3 years ago

tests are passing so im happy to merge this and get the new release ready. Following up on your earlier comment, do you want to add a warn if someone invokes n_jobs just to note that its in beta? Since we have the notebook and its not a breaking change i'm not sure its absolutely critical... presumably it shouldn't be too long before the required version of pygeos is released anyway?

martinfleis commented 3 years ago

GeoPandas 0.9 is expected within a month, pygeos 0.9 also soon (guessing from GitHub discussions).

Tests are passing because there's xfail decorator on the parallel one, which would otherwise fail.

darribas commented 3 years ago

I think a warning is in order, will try to get to it asap :)

darribas commented 3 years ago

OK @knaaptime this is good to go as far as Martin and I is concerned. I've moved the performance notebook to the notebooks folder and added a note on the documentation for area_interpolate that if n_jobs is not 1 you need edge pygeos/geopandas. Once that is released, we can remove the note or modify it with the version requirements.

Should we go ahead, merge and release so this makes it into the January PySAL release?