Open-EO / openeo-processes-python

A Python representation of (most) openEO processes
Apache License 2.0
11 stars 4 forks source link

Equi7 seems to use coordinate system with swapped x and y axis #197

Closed danielFlemstrom closed 2 years ago

danielFlemstrom commented 2 years ago

Current implementation in util.py uses the mathematical coord system as: bbox = [[x_min, y_min],[x_max, y_max]]

If we enter the coordinates like that we end up in AF (africa), but if we swap x and y we get an EU tile (that is also off, but at least in the correct sub-grid).

We suspect that Equi7 wants to have the geographic coordinate system where x and y are swapped. like this: bbox = [[y_min, x_min], [y_max, x_max]]

Anyway, we get strange results. When the answer is divided into the Equi7 tiles, the order according to the documentation is x_min, y_min, x_max, y_max = tile_bbox.
However the bounding box returned by gridder.get_tile_bbox_proj seems to be in metres? or something. There seems to be no parameter that synchs it to the source CRS....

This is the result we get (swapped x and y when asking for tiles, but creating the gridder with different EPSGs):

#                      west,   south,    east,   north
# EPSG:3006           13.31,   59.09    14.11,   59.44                      
# EPSG:4326          403050, 6551299   449522  6589385        

['EU300M_E048N024T6']
EPSG: 4326
ODC [[59.09, 13.31], [59.44, 14.11]]
EQ7 (4800000.0, 2400000.0, 5400000.0, 3000000.0)

['EU300M_E048N024T6']
EPSG: 3006
ODC [[6550350, 403050], [6590250, 449550]]
EQ7 (4800000.0, 2400000.0, 5400000.0, 3000000.0)

(Note that the first bbox has x and y swapped)

So whatever is stored in data.x and y cannot be cut out using that bbox. Did you not get any problems with this?

Maybe we should use lat / long instead of x and y as much as possible to stay out of trouble ?

Anyhow, we have a hard time seeing the use of splitting up the answer in these tiles. So making Equi7 optional seems to be our priority. It would be beneficial to have a confgurable gridder, where Equi7 is one (We may want to use or create a gridder for the swedish grid).

SerRichard commented 2 years ago

Hi Daniel!

Sorry I only just seen this, I have currently been working on updating this processes internally for the new repo metioned here.

Did you not get any problems with this?

Yeah, I have noticed that it is possible to get incorrect tiles from the equi7grid, but, the current bbox layout for me [[xmin, ymin], [max, ymax]] does seem to yield the correct tiles, I may double check this. However! In the updated version of the save result, we will not be using the equi7grid library. We did not like having the gdal dependency and the conda environment it requires, so for streamlining we will do the gridding from the shapefiles.

That being said, it would be good to get your input on how/ what you think the public facing implementation of save result would look like, as supporting multiple grids would be ideal!

danielFlemstrom commented 2 years ago

It seems to be a problem with the GDAL version. They swapped x and y in some version. In our code, i made equi7grid optional so I have to trust the equi7 team for that info. (They showed an example that we got the correct tile given the correct gdal version).

Anyway, good that you got rid of equi7grid :). Maybe save result can have an optional argument, pointing to a custom gridder ? (If people generally want to split up the result in tiles.) In our fork, i took a shortcut and if equigrid is not installed, it will not be used :)