Use round_fn to specify built-in round function for MVT

[iandees]

Use the built-in Python rounding function instead of the Decimal one.

[iandees]

This change makes quite a difference:

Coord	Format	Decimal Round	Built-in Round
1/1/1	json	377.4ms	359.2ms (-4.8%)
1/1/1	topojson	246.6ms	233.8ms (-5.2%)
1/1/1	mvt	1286ms	324.8ms (-74.7%)
1/1/1	mvtb	1444.4ms	376.2ms (-74%)
7/32/47	json	904.5ms	860.8ms (-4.8%)
7/32/47	topojson	588ms	562.4ms (-4.4%)
7/32/47	mvt	1615.8ms	617.8ms (-61.8%)
7/32/47	mvtb	1675ms	668.1ms (-60.1%)
12/1050/1522	json	3340.2ms	3228.2ms (-3.4%)
12/1050/1522	topojson	2283.1ms	2194.9ms (-3.9%)
12/1050/1522	mvt	5383.3ms	2386.7ms (-55.7%)
12/1050/1522	mvtb	6050.9ms	2692.5ms (-55.5%)
16/16812/24357	json	388.6ms	390.8ms (0.6%)
16/16812/24357	topojson	284.8ms	285.2ms (0.1%)
16/16812/24357	mvt	787.3ms	416.2ms (-47.1%)
16/16812/24357	mvtb	931.8ms	535.4ms (-42.5%)

[pnorman]

An improvement all a "round"

[pnorman]

Just to note, the behavior of round has changed with python version, and I don't see a minimum version specified here.

[iandees]

@pnorman where do you suggest we specify a minimum version?

[pnorman]

@pnorman where do you suggest we specify a minimum version?

https://github.com/tilezen/tilequeue#installation

I see that Python 2.7 is technically listed under https://github.com/tilezen/tilequeue/blob/master/setup.py#L22

The changes which I'm aware of which might impact the output are python 2.5 (or 2.6) which changed serialization of numbers to get 0.7 instead of 0.69999999, and python 3 which changes round. Either behavior is acceptable, it's just any tests which rely on exact numbers can fail.

[zerebubuth]

Either behavior is acceptable, it's just any tests which rely on exact numbers can fail.

You've hit the nail on the head. That's the reason why the default rounding function used in the MVT code is compatible between PY2 and PY3, although it turns out that's very slow and it seems unlikely that anyone would want to run it in production.

Speaking of which, although it seems unlikely that anyone would want anything other than the built-in round(), does anyone else think it's worthwhile making the choice of round function configurable in tilequeue?

[rmarianski]

Speaking of which, although it seems unlikely that anyone would want anything other than the built-in round(), does anyone else think it's worthwhile making the choice of round function configurable in tilequeue?

I don't think so. We might want to just change the encoder to default to the native round function, and modify the tests to use the slow consistent one.

[pnorman]

I don't think so. We might want to just change the encoder to default to the native round function, and modify the tests to use the slow consistent one.

The right thing to do probably is to avoid quality comparisons with floats, but this is probably the sane thing to do.

[zerebubuth]

The right thing to do probably is to avoid quality comparisons with floats

Indeed. In this case, the results of calling decode on the tile should be integers. Or so it seems to me from reading the coordinate decoding code.

The inputs to encode, on the other hand, are almost always floats and must be quantized. It's there that the rounding function makes a difference - even if only for the very small proportion of coordinates which have a fractional component of a half.

It's fairly easy to construct synthetic test cases which avoid the rounding behaviour. However, we have some "real data" test cases which exercise various paths through the validity-checking and -enforcing code, and I'm fairly sure that the rounding behaviour plays some part in that. Reducing to a minimal case which avoided "controversial rounding" might be a large amount of effort.

[iandees]

Can this be merged and documentation and/or tests updated separately?

[nvkelso]

@iandees It's merge-able as-is.

I've updated the readme documentation to note Python 2.7, but it was already in the setup.py so really this was friendly thing to state but not a merge blocker.