DeBroglie icon indicating copy to clipboard operation
DeBroglie copied to clipboard

Added AdjacentModel.NormalizeWeights and made SetFrequency/MultiplyFrequency optionally aware of RotatedTile

Open justonia opened this issue 7 years ago • 11 comments

When using the AdjacencyModel via AddSample I have found it very useful to be able to normalize the weights of each tile afterwards. I do not find the concept makes much sense in the OverlappingModel so I have not attempted to implement that.

Additionally, I found myself wanting to multiply the frequency of a tile and not have to manually specify every rotated/reflected variant of it, so I modified MultiplyFrequency in both models to support dereferencing a RotatedTile to it's base representation.

justonia avatar Oct 29 '18 03:10 justonia

Thanks for this. The changes are a good idea, but I'm going to spend a bit of time reviewing. I'd like to find a more principled way of dealing with generated tiles, as right now it seems I have to make the entire code base aware of them, which is annoying.

BorisTheBrave avatar Oct 29 '18 08:10 BorisTheBrave

Yea, the RotatedTile thing bit me a bit at first as I was using the API. I feed RotatedTiles directly into the AddSample calls since my input tilemaps are using rotation in many situations. I can see situations where I'd want to explicitly control the weights of a RotatedTile vs. its base Tile, but so far I've just needed the functionality in this PR.

Is it expected API usage to provide a RotatedTile to the AddSample API or is it a coincidence that it works currently?

justonia avatar Oct 29 '18 17:10 justonia

Yes, it's deliberate that you can input RotatedTile. The Tiled importer makes use of it. Though I always expected it to be pretty obscure.

On reflection, the general pattern I've been following is:

MyFunction(foo); // Does something with foo, doesn't deal with rotations of foo
MyFunction(foo, tileRotations); // Does something with foo and all it's rotations.

That's how AddSample and AddAdjacencyPair work. I think I'll change Set/MultiplyFrequency to work the same way.

Can you explain what your intention is behind NormalizeWeights, I'm a bit unclear on the use case.

BorisTheBrave avatar Oct 29 '18 19:10 BorisTheBrave

So in this example the collection of tiles in the bottom left are my "corpus". I use that tilemap to define adjacency pairs and valid placements.

image

And in my TileGenerator class I use these corpora as input for my generation process:

image

Without normalizing weights of each tile and its rotations, my output would be skewed towards the frequency of tiles in my input corpus, where instead I want to control frequencies of tiles independently of how they may be defined (via SetFrequency).

justonia avatar Oct 29 '18 19:10 justonia

Ok, so if I've understood correctly: NormalizeFrequencies(false) just sets every value in frequencies to 1 / n. NormalizeFrequencies(true) sets the combined frequency tile and all it's rotations to 1 / n, spreading the frequency in proportion to their original frequencies?

BorisTheBrave avatar Oct 29 '18 20:10 BorisTheBrave

NormalizeFrequencies(false), yes frequency[x] = x/n where n == totalFrequency.

For the NormalizeFrequency(true) case, the goal is the same but a tile and its rotations all "share" their chunk of x/n so that they are not over-represented in the final normalized frequencies.

justonia avatar Oct 29 '18 20:10 justonia

Ok, I see. That makes sense, but I think I prefer to fold that sharing behaviour into a new overload of SetFrequency.

BorisTheBrave avatar Oct 29 '18 20:10 BorisTheBrave

So if you did that the psuedocode equivalent for me would be (?):

foreach (tile) { model.SetFrequency(tile, 1, includeRotations:true); } 
model.NormalizeWeights()

justonia avatar Oct 29 '18 20:10 justonia

I was thinking that I'd set things up so that you don't need NormalizeWeights, as something like the following would be equivalent:

// NormalizeWeights(false)
for(var tile in model.Tiles)
  model.SetFrequency(tile, 1.0);
// NormalizeWeights(true)
for(var tile in model.Tiles)
  model.SetFrequency(tile, 1.0, tileRotation);

I.e. the idea is that passing a tileRotation argument means "be rotation aware".

BorisTheBrave avatar Oct 29 '18 20:10 BorisTheBrave

Ah yea got it, the actual value of frequency doesn't matter as long as every tile has the same value. When I think "normalized" I am biased towards [0,1].

justonia avatar Oct 29 '18 20:10 justonia

I've overhauled how rotations work a bit to standarize and hopefully simplify it. As part of that added overloads of SetFrequency and MultipleyFrequency which are "rotation aware", which I think is a useful generic operation.

You should now be able to get normalize frequencies behaviour as described in this comment.

BorisTheBrave avatar Nov 03 '18 23:11 BorisTheBrave