Prepare faster random land

[MM1nd]

Hi again,

World.random_land currently does two things: 1) Work some numpy magic to transform the ocean layer into a list of land indices. 2) Return a random pair of indices from that list.

The first part is really slow, the second part is nearly free. Thinking about it, as long as the ocean / land divide does not change, we don't need to repeat the first part every time we want a new sample. We can do (1) only once and then get arbitrarily many samples (2).

The implementation attached is not the nicest, but it has the advantage of not breaking the old World.random_land API. I thought that might be worth something, but this is up for discssion. I wanted to discuss this indepentently of the later application which is to use that concept to significantly speed up hydrology simulation (_watermap).

I also provided tests, that will be useful when actually messing with _watermap later.

So this PR doesn't actually do much but it prepares a later change. Again, let me know what you think.

Cheers

Alex

[coveralls]

Coverage increased (+0.01%) to 85.233% when pulling 58b0a141953e1f313067b06b9922cf2708fc68bc on MM1nd:prepare_faster_random_land into bdd349c6affb104b69798113afbe4b86b4eb24c6 on Mindwerks:master.

[coveralls]

Coverage increased (+0.8%) to 86.056% when pulling 58b0a141953e1f313067b06b9922cf2708fc68bc on MM1nd:prepare_faster_random_land into bdd349c6affb104b69798113afbe4b86b4eb24c6 on Mindwerks:master.

[codecov-io]

Codecov Report

Merging #243 into master will increase coverage by 0.08%. The diff coverage is 100%.

@@            Coverage Diff             @@
##           master     #243      +/-   ##
==========================================
+ Coverage   80.25%   80.33%   +0.08%     
==========================================
  Files          28       28              
  Lines        3884     3886       +2     
  Branches      768      767       -1     
==========================================
+ Hits         3117     3122       +5     
+ Misses        574      573       -1     
+ Partials      193      191       -2

Impacted Files	Coverage Δ
worldengine/common.py	82.89% <100%> (+2.37%)	:arrow_up:
worldengine/model/world.py	76.11% <100%> (+0.24%)	:arrow_up:

---

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update bdd349c...66895a4. Read the comment docs.

[psi29a]

I'm loving the additional test-coverage. :) I'm good with this if you are @ftomassetti

[MM1nd]

Obviously I'm still confused how branches work.

After the merge of #244 I merged those changes into this branch and now they show up in this PR. Shouln't I have done that?

[coveralls]

Coverage increased (+0.03%) to 85.255% when pulling 1c6817d65bf6b4b77267773fefbc07211cf15242 on MM1nd:prepare_faster_random_land into bdd349c6affb104b69798113afbe4b86b4eb24c6 on Mindwerks:master.

[coveralls]

Coverage increased (+0.03%) to 85.255% when pulling 1c6817d65bf6b4b77267773fefbc07211cf15242 on MM1nd:prepare_faster_random_land into bdd349c6affb104b69798113afbe4b86b4eb24c6 on Mindwerks:master.

[coveralls]

Coverage increased (+0.03%) to 85.255% when pulling 66895a47196c40e5623ef59632f13ae495a6b5e2 on MM1nd:prepare_faster_random_land into bdd349c6affb104b69798113afbe4b86b4eb24c6 on Mindwerks:master.

[MM1nd]

Hello again,

Admittedly I was getting a bit impatient, but it was right not to merge this. I made the same mistake in test and production code :( It's fixed now though.

The problem was that random_land, for compatibility with it's old API, returns a flat array of length num_samples*2 instead of an two dimensional array. This is not too complex, but it can get confusing and it did confuse me.

So we have to ask ourselves if API compatibility is worth that source of confusion. random_land is called in exactly one place outside of tests and that argument cuts in both directions. It doesn't hurt too much to break an API almost nobody calls, but it also doesn't hurt too much to have a confusing API nobody calls.

In any case I'm currently preparing a PR that actually uses this implementation for speedup. That's how I found the bug. Stay tuned.

Cheers Alex