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

PyProj transformers often reinstantiated #1915

Closed
Bob131 opened this issue Oct 28, 2021 · 5 comments · Fixed by #1918
Closed

PyProj transformers often reinstantiated #1915

Bob131 opened this issue Oct 28, 2021 · 5 comments · Fixed by #1918

Comments

@Bob131
Copy link

Bob131 commented Oct 28, 2021

Profiling an animation I've been fiddling with, I noticed about 10 seconds (~15% of the total run time) was spent in _safe_pj_transform:

         24631906 function calls (23998894 primitive calls) in 62.484 seconds

   Ordered by: cumulative time

   ncalls  tottime  percall  cumtime  percall filename:lineno(function)
...
     4660    0.025    0.000   11.467    0.002 geoaxes.py:117(transform_non_affine)
     4660    0.407    0.000   11.441    0.002 crs.py:336(transform_points)
...
     4660    0.016    0.000   10.998    0.002 crs.py:42(_safe_pj_transform)
     4660    0.025    0.000   10.859    0.002 transformer.py:471(from_crs)
     4660    0.033    0.000   10.813    0.002 transformer.py:298(__init__)
     4660    0.009    0.000   10.764    0.002 transformer.py:91(__call__)
     4660   10.724    0.002   10.746    0.002 {pyproj._transformer.from_crs}

Constantly creating new Transformer instances seems to be quite expensive. A quick hack improves things drastically:

sage: import functools
sage: ccrs.Transformer.from_crs = functools.cache(ccrs.Transformer.from_crs)
sage: cProfile.run('ani.save("/tmp/test.gif")', sort=pstats.SortKey.CUMULATIVE)
         24871283 function calls (24218669 primitive calls) in 51.364 seconds

   Ordered by: cumulative time

   ncalls  tottime  percall  cumtime  percall filename:lineno(function)
...
     4660    0.020    0.000    0.322    0.000 geoaxes.py:117(transform_non_affine)
...
     4660    0.147    0.000    0.301    0.000 crs.py:336(transform_points)
...
     4660    0.019    0.000    0.123    0.000 crs.py:42(_safe_pj_transform)
...
        1    0.000    0.000    0.003    0.003 transformer.py:471(from_crs)
        1    0.000    0.000    0.003    0.003 transformer.py:298(__init__)
        1    0.000    0.000    0.003    0.003 transformer.py:91(__call__)
...
        1    0.003    0.003    0.003    0.003 {pyproj._transformer.from_crs}

Is there any chance Cartopy could take care of caching transformers in a similar manner?

@QuLogic
Copy link
Member

QuLogic commented Oct 29, 2021

Yes, see also #1895.

@greglucas
Copy link
Contributor

@Bob131, a PR with that proposed fix would be most welcome!

We added in the cache inside the Cython routine to catch the segment transforms, but it looks like I forgot about the Transformer creation in the transform_points area of the code. See here for the other cache that we currently have:

@lru_cache(maxsize=4)
def _interpolator(src_crs, dest_projection):

@Bob131
Copy link
Author

Bob131 commented Oct 30, 2021

So I did prepare a PR, but the CLA put me off somewhat; I might leave it to someone else (sorry!).

I will mention a couple of things, though:

  • It turns out this pathological behaviour is specifically related to inline labels on the gridliner.
  • The only caller of _safe_pj_transform is CRS.transform_points, so instead of using lru_cache with an arbitrary size it seemed like a better idea to stash transformers in a WeakKeyDictionary on the CRS.
  • I wasn't able to properly run the test suite anyway; the matplotlib tests run on my machine look like the baseline images pre 2f6e5a0, so almost every test fails.

@greglucas
Copy link
Contributor

:( That is a bummer you're put off by the CLA. I don't think you're alone there either.

I took your idea above and created #1918, can you check to make sure that works for your case?

@QuLogic
Copy link
Member

QuLogic commented Oct 31, 2021

So I did prepare a PR, but the CLA put me off somewhat; I might leave it to someone else (sorry!).

Note, it's more of a DCO than the more onerous CLAs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants