SciTools/cartopy

Port to use pyproj v2

Closed this issue · 12 comments

dopplershift commented

We should consider replacing our own compiled Cython extension that links directly to libproj with pyproj v2 (as initially mentioned in #1252). Benefits I see:

  • Eliminate some packaging issues with users needing to separately compile and install libproj
  • We sidestep the need to update our compiled code to account for API deprecations and changes (#1140, #1252)
  • Eliminate the needs to maintain our Cython code

Cons:

  • Takes work
  • Might require upstream modification (I have no idea what our compiled wrappers do that we can't do through the pyproj python interfaces)

Given that both pyproj and PROJ are active now, it would seem to benefit us to offload this functionality upstream.

snowman2 commented

Additional benefits:

  • remove need for pyepsg
  • add support for projections defined by WKT, spatial reference ID, CF grid_mapping, etc..
  • would resolve #1023
  • pyproj has wheels
  • Can construct CRS from rasterio.crs.CRS & geopandas CRS (now uses pyproj.CRS)
snowman2 commented

I have a start here: master...snowman2:pyprojv2

But there are a lot of test failures. I could definitely use some help on that (or feel free to take what I have and continue working on it).

This is the main one that pops up in a lot of places:

  • pyproj.exceptions.ProjError: Error creating Transformer from CRS.: (Internal Proj Error: proj_create_operations: Source and target ellipsoid do not belong to the same celestial body) (Related to: #1252 (comment))
atwilso commented

@snowman, what's the current state of this work? I'm interested in helping untangle the dependency chain that stands in the way of Cartopy wheels but I don't know whether I should be looking at this or at Shapely first.

snowman2 commented

#1644 needs to be resolved before the transition to pyproj can be completed.

atwilso commented

Thank you! That gives me a place to start investigating.

  • 👍1
greglucas commented

Note that this path would allow us to avoid the rewrite within Cartopy for PROJ8 that is being undertaken in #1752 and let pyproj handle the PROJ interfacing instead.

One major downside of all of this that I'm seeing is that somehow the tests are taking significantly longer in both situations (~10x slowdown, going from 2 minutes to 20 minutes on CI). Is there something in the new header that is inherently slower?

snowman2 commented

Based on what I saw in #1252, cartopy uses the global context. This makes things much faster, but it is not thread-safe.

By default, pyproj creates a context for each object for thread safety reasons. The downside is the performance hit you run into. That said, you can tell pyproj to use the global context ref and see some significant performance improvements.

PROJ 8.1+ will include this change that will significantly speed things up without using the global context.

  • 👍1
  • ❤️1
snowman2 commented

This might also impact things: OSGeo/PROJ#2734

greglucas commented

Thanks for the summary, @snowman2! That makes it sound like the slowdown is definitely not a major issue moving forward. I'm still in favor of this solution. Would you be willing to open up a PR with that branch you had before? I'll try and ping others for a review on the celestial body/PlateCarree PR I opened.

  • 🚀1
snowman2 commented

Seems even with the global context it is slower. I wonder if this has anything to do with it: https://pyproj4.github.io/pyproj/stable/advanced_examples.html#optimize-transformations

snowman2 commented

Also wondering how the speed can compares with PROJ 8.1 (current master) 🤔