Improve performance of LitPop

[peanutfun]

Changes proposed in this PR:

• Speed up country-code detection in LitPop by assuming the data is gridded (it is)
• Streamline gridpoints_core_calc to use numpy array tools and broadcasting, and avoid copying arrays
• Add option to pass region ID to LitPop.from_shape. This avoids country-code detection.

PR Author Checklist

• Read the Contribution Guide
• Correct target branch selected (if unsure, select develop)
• Descriptive pull request title added
• Source branch up-to-date with target branch
• Documentation updated
• Tests updated
• Tests passing
• No new linter issues
• Changelog updated

PR Reviewer Checklist

• Read the Contribution Guide
• CLIMADA Reviewer Checklist passed
• Tests passing
• No new linter issues

[chahank]

Looks all very reasonable! Any estimates of the gained computation time? Also, changelog needs to be updated. Maybe also the documentation to point people towards how to use custom shapes correctly.

[peanutfun]

Any estimates of the gained computation time?

I was afraid you would ask that 😬 I can come up with some benchmarks tomorrow. The most important improvement is from using get_country_code(gridded=True). The actual LitPop computations are very fast, and avoiding copies does not actually help much with that. In my use case, LitPop was occupied for some 50s, 90% of which were spent with get_country_code. The other large bottleneck I found was geopandas.points_from_xy, but that resolved itself after upgrading from shapely v1.8 to v2.0.

So, tldr: from_shape can be by a factor 10 faster with this PR. But I'll run a benchmark just to be sure.

[peanutfun]

Maybe also the documentation to point people towards how to use custom shapes correctly.

Not sure what you mean by that. How are people using it "incorrectly" in your perspective?

[chahank]

Not sure what you mean by that. How are people using it "incorrectly" in your perspective?

Sorry for being unclear. I was referring to the original issue which was about how to use arbitrary shapes with LitPop. This functionality was clearly not obvious, so a documentation entry could help.

[peanutfun]

Ah, sorry, you were referring to #715. I'll see what I can do.

[peanutfun]

So here are some benchmark results.

LitPop.from_countries

(basically no change)

on develop

on this branch

LitPop.from_shape

(time spent in get_country_code dropped from 48% to 13 %)

on develop

on this branch

[peanutfun]

Sorry for being unclear. I was referring to the original issue which was about how to use arbitrary shapes with LitPop. This functionality was clearly not obvious, so a documentation entry could help.

I moved the Issue to Discussions/Q&A and marked my workaround as answer, see #723

[chahank]

So here are some benchmark results.

Small improvements, but certainly useful! In particular since from_countries is likely not to be used too often as country data is available on the API, but the from_shape is more important for tailored applications.

[chahank]

This is really nitpicking, but result is a terrible variable name ^^.

[peanutfun]

True, but result_array was even worse. Do you have something better? The name can also be omitted here:

Suggested change

	result : np.array of same shape as arrays in data_arrays
	np.array of same shape as arrays in data_arrays

[chahank]

Well, the variable name still would be there. Maybe convoluted_array since this is essentially doing a convolution? Or rescaled_product ?

[peanutfun]

How about leaving out the result variable and storing everything in the data variable? I think that the new variable definition actually is not needed.

[chahank]

Also fine. Although data is not a better name than result.

[peanutfun]

Then I'll do that. My goal is not to refactor this function 😅

[peanutfun]

Done in 620d346

[peanutfun]

Small improvements, but certainly useful!

Yes, I think I mixed something up. The major improvement was upgrading to shapely v2.0 on my side.

[peanutfun]

@chahank Ready to merge from my side!