Open-EO / openeo-earthengine-driver

openEO back-end driver for Google Earth Engine.
Apache License 2.0
21 stars 7 forks source link

"Inverted" results for load_collection with geoJSON (v0.4.1) #40

Closed jneijt closed 4 years ago

jneijt commented 4 years ago

I've been using the option to limit the data loaded from a collection using a geoJSON. Until recently this has been working perfectly, but now I noticed that some geoJSONs seem to return an "inverted" result. Basically, the process graph doesn't return the area within the geoJSON, but the region outside of the polygon.

The attached process graphs are simplified examples, one working as expected (luzern.txt) and one returning the inverted result (biel.txt). The results for this issue were generated using the openEO web editor and correspond to the results I get using the JS client locally, all run on the 0.4.1. version of the backend.

Result as expected: luzern_result luzern.txt

Unexpected result: biel_result biel.txt

Any ideas why this is happening? Could it be something in the process graph?

m-mohr commented 4 years ago

Thank you for the report.

This is interesting, the difference is Polygon vs. MultiPolygon and I can reproduce the issue in 0.4. Unfortunately, I couldn't figure out the reason yet. In 1.0, it's working as excepted with the process graph converted to 1.0 syntax (see below). So is it still important to fix this in 0.4 or is 1.0 enough?

{
  "process_graph": {
    "dc": {
      "process_id": "load_collection",
      "arguments": {
        "id": "COPERNICUS/S2_SR",
        "spatial_extent": {
          "type": "Polygon",
          "coordinates": [
            [
              [
                7.2011030978273,
                47.1329319703602
              ],
              [
                7.21202247943189,
                47.1389460034675
              ],
              [
                7.22952642223754,
                47.1448371591858
              ],
              [
                7.24030045642916,
                47.1472046803482
              ],
              [
                7.24869117034048,
                47.1545731976403
              ],
              [
                7.24479607526853,
                47.1572140052941
              ],
              [
                7.2274632615245,
                47.1538416088261
              ],
              [
                7.24700377491079,
                47.1655127797531
              ],
              [
                7.26059735128467,
                47.1682147873431
              ],
              [
                7.27045465472884,
                47.1682494761179
              ],
              [
                7.27175703222776,
                47.1712398931332
              ],
              [
                7.29297407142879,
                47.1770418001956
              ],
              [
                7.2975561081985,
                47.1665512874267
              ],
              [
                7.31271059721007,
                47.1606607670716
              ],
              [
                7.2981182739033,
                47.1563012124568
              ],
              [
                7.30009241157168,
                47.1524281893199
              ],
              [
                7.29007709446903,
                47.1467363166575
              ],
              [
                7.28366635727544,
                47.1389460645729
              ],
              [
                7.27463676153406,
                47.1392594124052
              ],
              [
                7.26247662685905,
                47.1246081818955
              ],
              [
                7.25253829537625,
                47.1260334039064
              ],
              [
                7.24532221853044,
                47.1296189659793
              ],
              [
                7.23354782043843,
                47.1307213680268
              ],
              [
                7.21551099700624,
                47.1271506710711
              ],
              [
                7.20696882713802,
                47.1290607890708
              ],
              [
                7.2011030978273,
                47.1329319703602
              ]
            ]
          ]
        },
        "temporal_extent": [
          "2020-04-14",
          "2020-04-20"
        ],
        "bands": [
          "TCI_R",
          "TCI_G",
          "TCI_B"
        ]
      }
    },
    "reduce": {
      "process_id": "reduce_dimension",
      "arguments": {
        "data": {
          "from_node": "dc"
        },
        "reducer": {
          "process_graph": {
            "min": {
              "arguments": {
                "data": {
                  "from_parameter": "data"
                }
              },
              "process_id": "min",
              "result": true
            }
          }
        },
        "dimension": "t"
      }
    },
    "apply": {
      "process_id": "apply",
      "arguments": {
        "data": {
          "from_node": "reduce"
        },
        "process": {
          "process_graph": {
            "2": {
              "process_id": "linear_scale_range",
              "arguments": {
                "x": {
                  "from_parameter": "x"
                },
                "inputMin": 0,
                "inputMax": 255,
                "outputMax": 255
              },
              "result": true
            }
          }
        }
      }
    },
    "save": {
      "process_id": "save_result",
      "arguments": {
        "data": {
          "from_node": "apply"
        },
        "format": "PNG",
        "options": {
          "red": "TCI_R",
          "blue": "TCI_B",
          "green": "TCI_G"
        }
      },
      "result": true
    }
  }
}
jneijt commented 4 years ago

Thanks for the quick response. It indeed looks like it depends on using MultiPolygon vs Polygon. I first thought this was a coincidence as I had seen other locations which seemed to be using a single Polygon but as it turns out these also include a very small (and in the result therefore almost invisible) second Polygon. And removing the second Polygon breaks them, so I can confirm that this is the reason. Interestingly this has been working correctly until a couple weeks/months ago in 0.4.1 as well.

Anyway, for us it should be enough if it works in 1.0. Until now I didn't migrate to 1.0 as not all processes were supported yet, but we can work with the current results on 0.4.1 until they are, that's fine :-) And next week I should have some time to test some more with 1.0.

Thanks for the support!

m-mohr commented 4 years ago

I can't really tell why this changed, there were no changes in related code after August 2019, so it may be a change on Google's side or an issue in dependencies. Although it's strange that 1.0.0 works with mostly unchanged code regarding GeoJSON filtering, but I think the drivers work on different GEE API versions...

What's missing in 1.0.0 for you? I thought we have everything in 1.0.0 that we had in 0.4.

By the way, I have a JS Client version for 1.0.0, too, but it's still a bit "beta", especially regarding new features (mostly OpenID Connect and log files).

jneijt commented 4 years ago

I think I just missed one re-name in my last quick compatibility check and as it wasn't urgent for me I just stayed on 0.4. But you are right, it should all be there, thanks for the heads up.

Using the process_graph above I am currently getting a "Maximum Call Stack Exceeded" in the Web Editor. I guess these are just due to the active development on the backend at the moment and not an error in the process_graph?

m-mohr commented 4 years ago

That's very likely an error in openeo-js-processgraphs, I fixed something related some days ago and probably haven't updated the public instances yet. The process graph ran perfectly fine on my local instance. :-)

jneijt commented 4 years ago

Perfect, thanks :-) I'll continue migrating to 1.0 and let you know in case I run into any issues again. From my side we can close this issue, thanks for the help. :-)