random -> numpy.random

[tcld]

Here the replacement of all uses of the random-module with respective numpy-methods.
Since this is kind of a break with save-compatibility and...everything, I guess, I also took the freedom to include some small changes so they aren't necessary in the future - I will add comments to the code to point them out and explain them.

Notes:

1. This is based on Make use of Numpy-Arrays #138 .
2. This does not pass the ancient-map tests (since that one uses RNG to determine its outcome)

[tcld]

This was very much unnecessary unless the RNG is expected to be of very dubious quality. I chose to simplify it.

[tcld]

I might change some small things about this before uploading the new blessed ancient maps. Once that is done the tests should be fine again.

[tcld]

It would be nice if somebody could look over the code; I think I touched all that is necessary, but this will need some polish, I presume.

[tcld]

The method random_point() isn't used anymore so I removed it (and this test). It was an almost trivial method anyway.

[ftomassetti]

It looks good to me

[tcld]

I don't like the use of this fixed number at all. Does somebody have a better idea? sys.maxint (or whatever it is called) is not available for Python 3; sys.maxsize returns a number that is not compatible with numpy (which takes a uint32).

[tcld]

After all this I ended up here:

It might not be immediately visible but everything except the mountains is the same (see the middle island). Before the changes to the RNG there were a lot more (small) differences. Does anybody have an idea why the mountains differ?

PS: I am referring to differences between Python 2 and 3.

[ftomassetti]

the mountain generation is quite complicate. We have a mask to avoid overlapping mountains and a system to determine the size of each single mountain which is based on the elevation in the area. I am not sure if it is using the world seed.

[ftomassetti]

it seems we are not using any seed for the mountains BUT we are using it for the other masks. Basically we assign each pixel to some masks, ~one for biome, and then we draw that pixel accordingly. If any mask change it could affect how mountains are drawn

[tcld]

I assume you are talking about this:

def _draw_a_mountain(pixels, x, y, w=3, h=3):
    # mcl = (0, 0, 0, 255)  # TODO: No longer used?
    # mcll = (128, 128, 128, 255)
    mcr = (75, 75, 75, 255)
    # left edge
    for mody in range(-h, h + 1):
        bottomness = (float(mody + h) / 2.0) / w
        leftborder = int(bottomness * w)
        darkarea = int(bottomness * w / 2)
        lightarea = int(bottomness * w / 2)
        for itx in range(darkarea, leftborder + 1):
            pixels[x - itx, y + mody] = gradient(itx, darkarea, leftborder,
                                                 (0, 0, 0), (64, 64, 64))
        for itx in range(-darkarea, lightarea + 1):
            pixels[x + itx, y + mody] = gradient(itx, -darkarea, lightarea,
                                                 (64, 64, 64), (128, 128, 128))
        for itx in range(lightarea, leftborder):
            pixels[x + itx, y + mody] = (181, 166, 127, 255)  # land_color
    # right edge
    for mody in range(-h, h + 1):
        bottomness = (float(mody + h) / 2.0) / w
        modx = int(bottomness * w)
        pixels[x + modx, y + mody] = mcr

Yes, this is complicated.^^ I worked through it a couple of days ago, but there is no directly visible use of something random (and the RNG in draw_ancientmap() is already replaced with a numpy one).

[tcld]

I will take another look at it tomorrow. We are very close to the goal already.^^

[tcld]

I did one more thing:

So the mountains are clearly different; but I think the background-colors might be slightly shifted, too. (Maybe I just misunderstood how ImageMagick's compare works).
Anyway, it's a nice comparison.

[tcld]

I think I found the cause for the remaining problems:

def _mask(world, predicate, factor):
    _mask = [[False for x in range(factor * world.width)] for y in
             range(factor * world.height)]
    for y in range(factor * world.height):
        for x in range(factor * world.width):
            xf = int(x / factor)
            yf = int(y / factor)
            if predicate((xf, yf)):
                v = len(
                    world.tiles_around((xf, yf), radius=1,
                                       predicate=predicate))
                if v > 5:
                    _mask[y][x] = v
    return _mask

Apparently this map only stores integers in Python 2 (???) but floats in Python 3. It was going to be replaced by numpy anyway, I hope that will fix things.

EDIT: Maybe Python 2 does not automatically typecast at this position?

_mask[y][x] = v / 4   # v is an integer

EDIT2: That last assumption seems to be right. I will just add a typecast, assuming that was the intended behaviour. If not, we can change that later.

Python 2.7.6 (default, Jun 22 2015, 17:58:13) 
[GCC 4.8.2] on linux2
Type "help", "copyright", "credits" or "license" for more information.
>>> 5/4
1

Python 3.4.3 (default, Oct 14 2015, 20:28:29) 
[GCC 4.8.4] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> 5/4
1.25

[tcld]

I could use some help with the pyflakes- and manifest-tests since I don't think my changes make them fail.

Mindwerks/worldengine-data#5 will take care of the problems with ancient maps.

Other than the failing tests, this code is ready and could be reviewed, if anybody has the time.

[tcld]

Not sure which behaviour was the intended one here. The ancient map output looks different, I think the mountains are spread a little thinner for the old behaviour int(v/4).

[ftomassetti]

Restarting tests after the changes to worldengine data

[tcld]

I consider this passed.^^ It would be nice if this could be the next merge so I can cross this off my list and move on to finishing another branch (instead of having to come back here and rebase/fix things).

[ftomassetti]

I think it could be nice to have methods such as random_point or perhaps random_land_point but we could add it later if needed

[tcld]

It was only used once. Removing it allowed for another numpy-optimization; so for now this function would have been dead code (and since it is fairly simple anyway, rewriting it would be easy).

EDIT: I am talking about this optimization:
https://github.com/Mindwerks/worldengine/pull/146/files#diff-c341fcae944ab9ce9e38ea34a0165badR337

It is a very important one in my opinion.

[ftomassetti]

Looks good to me. I would just leave a possibility to @psi29a to comment on it before merging but it is agreed that this one should be merged soon.

A lot of great work on this one, thanks a lot!

[tcld]

I hope I took proper care of your comments.

[ftomassetti]

I am satisfied, as I said I would just leave @psi29a the time to comment, if he wishes to do so. If not this is good to go for me.

[ftomassetti]

Let's start merging stuff

[psi29a]

No need to wait on me... I'm in the middle of moving so I'm MIA for a bit.