[WIP]: Optimized raster-based weights builder

[MgeeeeK]

This pull request adds a new optimized weight builder for the raster interface.

Main takeaway:

• Sparse matrix is build using rows and cols values which are generated iteratively.
• The parallel implementation is based on the crand engine from esda/#116, though this work uses threading backend.
• This pr also adds functionality to add higher_order neighbors, this is done using 2 ways:
  • Firstly, 2 arguments are added to the builder method (k=int and include_nas=boolean). k is used for specifying the kth order and include_nas=True is used for including higher-order neighbors even if lower-order neighbors are missing.
  • higher_order functionality is based on the suggestion provided by Martin in this issue libpysal/#issue313. This also adds diagonal elements that are first converted to zero and then these zero elements are removed in the last step. But this changes the sparsity of the matrix!.
  • If include_nas is True then the method simply uses modular arithmetic to add higher-order neighbors. (with one caveat explained earlier and in this thread libpysal/#328)

Here is the plot showing difference between two approaches.

[codecov]

Codecov Report

Merging #343 (1804960) into master (5312a3a) will increase coverage by 0.7%.
The diff coverage is 54.1%.

@@           Coverage Diff            @@
##           master    #343     +/-   ##
========================================
+ Coverage    80.3%   80.9%   +0.7%     
========================================
  Files         117     117             
  Lines       12163   12044    -119     
========================================
- Hits         9765    9748     -17     
+ Misses       2398    2296    -102     

Impacted Files	Coverage Δ
libpysal/weights/raster.py	61.3% <48.5%> (-22.1%)	⬇️
libpysal/weights/contiguity.py	78.6% <100.0%> (-0.5%)	⬇️
libpysal/weights/tests/test_contiguity.py	95.7% <100.0%> (-0.1%)	⬇️
libpysal/weights/tests/test_raster.py	97.2% <100.0%> (ø)
libpysal/io/iohandlers/csvWrapper.py	95.2% <0.0%> (-1.7%)	⬇️
libpysal/io/geotable/utils.py	60.0% <0.0%> (-1.3%)	⬇️
libpysal/io/util/tests/test_shapefile.py	98.3% <0.0%> (-0.8%)	⬇️
libpysal/io/iohandlers/gal.py	60.6% <0.0%> (-0.8%)	⬇️
... and 102 more

[sjsrey]

I can see the logic of offering the option for include_nas. So I'm +1 on that. I'm wondering if there is a more informative name for the argument, possibly? I'm not clear on what nas stands for?

[MgeeeeK]

Sure! we can rename that argument. What do you guys suggest @darribas @slumnitz?
I think we assigned that as the temporary name because the functionality was not fully finalized.

[darribas]

Almost there!!! Mostly issues of documentation and style, the only more substantive bit is to ensure the id_order of the weights we build is relatable to the DataArray

[darribas]

Just a couple minor bits to add, only docs and tests and I think this is good to ship!

[slumnitz]

@MgeeeeK this looks great and I think we are super close to merging. The only thing remaining is to test a couple more lines. I resolved all of @darribas comments that where addressed and looked great. There are a couple minor comments still standing that slipped through the radar and are still open (even if marked outdated). Once these are added I am happy to get this merged. Thank you so much for your hard work this and last year!

[darribas]

I've resolved a couple of the standing threads, they look good. I'm not sure test_da2WSP is being fully tested. I can see rook and queen are now both tested, but it's always in the multi-core context. Is it a different bit of code that runs it on single core? If not, it's fine, but if it is, we should test that too.

[MgeeeeK]

Is it a different bit of code that runs it on single core?

No, multi-thread is extension of the base constructor but still I'll add another test comparing results of multi-threaded and single-threaded builder.

[darribas]

I think this is good to go as far as I'm concerned. @slumnitz ? If both give it green light, I think @sjsrey or us can merge?

[slumnitz]

Let's get this merged then! :)