Round gridspec tilesize to fix floating point precision issue

[alexgleith]

Fixes an issue where small resolutions (i.e., 0.003) would lead to crazy coordinates in geoboxes for a gridspec, due to floating point issues.

For example, if you create a gridspec like this, the transform has a rounding error with a very small number added at the end:

from odc.geo.gridspec import GridSpec, XY

WGS84GRID30 = GridSpec("EPSG:4326", tile_shape=(5000, 5000), resolution=0.0003, origin=XY(-180, -90))

tile = (50, 50)
geobox = WGS84GRID30.tile_geobox(tile)

geobox.affine

Affine(0.0003, 0.0, -105.00000000000001, 0.0, -0.0003, -13.500000000000014)

The issue is caused when calculating the tile size (the width and height of the tile, in units of the gridspec), which works like this:

from odc.geo.types import shape_, res_

tile_shape = shape_((5000, 5000))
resolution = res_(0.0003)

tile_shape.x * abs(resolution.x)

1.4999999999999998

The proposed fix is to round these numbers at 12 decimal places, which is enough to fix this problem and I hope doesn't cause issues in other areas. New test included an all other tests pass.

[github-actions]

🚀 Deployed on https://66fb650776544589e7646273--odc-geo-docs.netlify.app

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 95.44%. Comparing base (ea04613) to head (ff8f323).

Additional details and impacted files

@@           Coverage Diff            @@
##           develop     #180   +/-   ##
========================================
  Coverage    95.44%   95.44%           
========================================
  Files           31       31           
  Lines         5534     5534           
========================================
  Hits          5282     5282           
  Misses         252      252           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[Kirill888]

Why?

[alexgleith]

Why?

Why the PR?

See the test output without the fix.

[Kirill888]

The proposed solution is not a robust one. Issue it’s trying to solve is not explained in the pr and it feels like the real problem is some place else. Version updates are typically done as a separate commit in this repo

[alexgleith]

The proposed solution is not a robust one.

I agree. Not sure if there's a better approach, though.

feels like the real problem is some place else

I looked through the code and debugged pretty deep. This is where the problem is caused.

Issue it’s trying to solve is not explained in the pr

Ok, I'll explain it. Apologies...

Version updates are typically done as a separate commit in this repo

Ok, I'll remove the version update.

[alexgleith]

Text updated to explain the reasoning @Kirill888.

[Kirill888]

Unrelated build fixes should be a separate commit.
There is a robust way of rounding affine matrix already
https://odc-geo.readthedocs.io/en/latest/_api/odc.geo.math.snap_affine.html#odc.geo.math.snap_affine

We can NOT keep adding rounding in random places to support specific issues caused by use of EPSG 4326

[omad]

@alexgleith

Is there an actual problem, other than that the numbers look funny?

[alexgleith]

Is there an actual problem, other than that the numbers look funny?

Yeah, the gridspec drives the input and output grid for an analysis, so the coordinates of the geotiff that's being produced will be screwy too.

[alexgleith]

There is a robust way of rounding affine matrix already

But the fundamental issue is not the affine, it's the GridSpec, which is producing bad GeoBoxes with bad Affines.

And the bad GridSpec is caused by the incorrect rounding on the tile size. It could be fixed after the fact, but that feels like hiding the underlying problem, to me.

Unrelated build fixes should be a separate commit.

Ok, fair. I can split that noise out.

[alexgleith]

So, is there a better way to do this? I'd really like to not need to maintain my own function for creating a gridspec because we can't handle digital numbers here.

This sucks!