Mindwerks / worldengine

World generator using simulation of plates, rain shadow, erosion, etc.
MIT License
988 stars 129 forks source link

random -> numpy.random #146

Closed tcld closed 8 years ago

tcld commented 8 years ago

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 https://github.com/Mindwerks/worldengine/pull/138 .
  2. This does not pass the ancient-map tests (since that one uses RNG to determine its outcome)
tcld commented 8 years ago

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 commented 8 years ago

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.

ftomassetti commented 8 years ago

It looks good to me

tcld commented 8 years ago

After all this I ended up here: ancientmap_48956 ancientmap_48956

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 commented 8 years ago

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 commented 8 years ago

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 commented 8 years ago

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 commented 8 years ago

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

tcld commented 8 years ago

I did one more thing: diff 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 commented 8 years ago

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 commented 8 years ago

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

https://github.com/Mindwerks/worldengine-data/pull/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.

ftomassetti commented 8 years ago

Restarting tests after the changes to worldengine data

tcld commented 8 years ago

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 commented 8 years ago

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 commented 8 years ago

I hope I took proper care of your comments.

ftomassetti commented 8 years ago

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 commented 8 years ago

Let's start merging stuff

psi29a commented 8 years ago

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