Routing refactor PEP

[emlys]

Description

Fixes #1440

Corresponding users guide updates in this PR: natcap/invest.users-guide#166 Since that's going into the release branch separately, I don't think this PR needs to wait for it.

Almost all of this work has already been reviewed in PRs into feature/routing-refactor.

test updates

I updated expected values in a SDR test, by amounts attributable to floating point imprecision changes due to the new implementation.

I also had to update expected values in an NDR test by a larger amount. This is due to changes from #1737 - similar changes were made there to other expected values.

the docker build

To get the docker container build passing, I had to install the libgdal-dev package for debian, and update setup.py to pass in the appropriate -I (include) directory to the compiler. Unlike everywhere else, we're using the system package manager and not conda to install the gdal binaries. This means that (at least on debian stable) we're stuck at gdal 3.6.2. I think this is fine - since we are linking dynamically, when the wheel is installed it should use whatever version of gdal is available in that environment. The gdal version in the build environment just needs to have a compatible API. If this proves not to work for some reason, we can look into using debian:sid or conda to install a newer gdal version.

performance

I did a final check of the sdr_core.calculate_sediment_deposition runtime using the SDR sample data. With n=100 repetitions and skipping the pygeoprocessing.new_raster_from_base calls (which seem to introduce a lot of variation), the old implementation averaged 0.67 seconds on my laptop. The new implementation averaged 0.48 seconds, a 28% improvement!

manylinux build changes (a wild goose chase)

In #1730 our wheel ended up being compatible with manylinux_2_34 (systems with glibc>=2.34). Merging in the routing C++ extensions bumped that up to manylinux_2_39 (systems with glibc>=2.39). That really threw me for a loop because there is no manylinux_2_39 docker container.

But as I understand it now, the manylinux docker container isn't actually needed. It's the auditwheel repair step that produces a manylinux-compatible wheel - this can be run on any linux system. The manylinux docker containers are useful if you want to support older systems (as determined by the glibc version). If we are okay with requiring this relatively recent version, we can avoid a whole lot of trouble with getting a more-backwards-compatible libgdal.

So, I removed the whole docker container step, and now we're building a manylinux_2_39 wheel on the regular ubuntu runner.

Checklist

• Updated HISTORY.rst and link to any relevant issue (if these changes are user-facing)
• Updated the user's guide (if needed)
• Tested the Workbench UI (if relevant)

[emlys]

I was seeing a warning about the mismatched case in FROM and as

[phargogh]

Before getting into the meat of the PR, I just wanted to respond to each of the points in the PR posting here.

RE: the docker build and GDAL version available there, I think this is a great approach for us for now, until we know for sure that we need to sink more time into it. Debian:stable is usually a good baseline, so as long as the APIs remain compatible (fingers crossed!) as the versions advance, I think we should be OK. Not sure what this will mean come GDAL 4.0, but we will see!

RE: performance, CONGRATS ! I can't wait to try it out on a big watershed on one of the Sherlock machines.

RE: manylinux, nice sleuthing with the container vs auditwheel. Especially since linux is by far the easiest OS on which to compile things, I think we should be a-ok with requiring a relatively recent version of glibc until we know otherwise.

THANK YOU for all your work on this and on to the code!

[phargogh]

I didn't take a super close look at the C++ content because I recognized it from prior reviews, and since the checks are all passing (with the slightly modified expected results, still numerically close), I am confident they are doing what we expect :)

My one question/comment is about the D8/MFD input. Since it's duplicated across 3 models, should this be abstracted out to spec_utils?

Otherwise, this looks great and I'm happy to merge!

[phargogh]

Since we have this defined in 3 models, would it be worth abstracting the options out into an object in spec_utils?

[emlys]

Oh good idea! I moved it to spec_utils and also renamed the key to flow_dir_algorithm to be more specific

[phargogh]

Thanks @emlys ! 🎉