gama-platform / gama.old

Main repository for developing the 1.x versions of GAMA
GNU General Public License v3.0
304 stars 99 forks source link

Problem related to GAMA Client/Server where Points of LinearRing do not form a closed linestring #3433

Closed agrignard closed 2 years ago

agrignard commented 2 years ago

Describe the bug When making the link between GamaServer and a VCity (Javascript client) I notice an error thrown by GAMA related to JTS. This issue is also written here https://github.com/VCityTeam/UD_ReAgent_ABM/issues/11

To Reproduce Steps to reproduce the behavior:

  1. Run Gama in headless mode (from eclipse run msi.gama.headless.id4_full.launch
  2. Clone this repo https://github.com/gama-platform/gama.client
  3. Clone this repo to get the concerned model https://github.com/VCityTeam/UD_ReAgent_ABM
  4. Be sure to write the right modelPath to your GAMA model in gama_client.js
  5. Open in a browser gama.client/JavaScript/index.html
  6. . After few iteration (not always the same number) GAMA is sending this error:

java.lang.IllegalArgumentException: Points of LinearRing do not form a closed linestring
    at org.locationtech.jts.geom.LinearRing.validateConstruction(LinearRing.java:93)
    at org.locationtech.jts.geom.LinearRing.<init>(LinearRing.java:88)
    at org.locationtech.jts.geom.GeometryFactory.createLinearRing(GeometryFactory.java:382)
    at org.locationtech.jts.geom.util.GeometryEditor$CoordinateSequenceOperation.edit(GeometryEditor.java:327)
    at org.locationtech.jts.geom.util.GeometryEditor.editInternal(GeometryEditor.java:160)
    at org.locationtech.jts.geom.util.GeometryEditor.edit(GeometryEditor.java:133)
    at org.locationtech.jts.geom.util.GeometryEditor.editPolygon(GeometryEditor.java:178)
    at org.locationtech.jts.geom.util.GeometryEditor.editInternal(GeometryEditor.java:152)
    at org.locationtech.jts.geom.util.GeometryEditor.edit(GeometryEditor.java:133)
    at org.locationtech.jts.geom.GeometryFactory.createGeometry(GeometryFactory.java:664)
    at msi.gama.metamodel.topology.projection.Projection.inverseTransform(Projection.java:144)
    at msi.gama.headless.common.SaveHelper.buildFeature(SaveHelper.java:223)
    at msi.gama.headless.common.SaveHelper.buildGeoJSon(SaveHelper.java:323)
    at msi.gama.headless.listener.GamaWebSocketServer.onMessage(GamaWebSocketServer.java:205)
    at org.java_websocket.server.WebSocketServer.onWebsocketMessage(WebSocketServer.java:712)
    at org.java_websocket.drafts.Draft_6455.processFrameText(Draft_6455.java:986)
    at org.java_websocket.drafts.Draft_6455.processFrame(Draft_6455.java:910)
    at org.java_websocket.WebSocketImpl.decodeFrames(WebSocketImpl.java:401)
    at org.java_websocket.WebSocketImpl.decode(WebSocketImpl.java:233)
    at org.java_websocket.server.WebSocketServer$WebSocketWorker.doDecode(WebSocketServer.java:1114)
    at org.java_websocket.server.WebSocketServer$WebSocketWorker.run(WebSocketServer.java:1086)
Java error: illegal argument

Expected behavior No crash from Gama Server

Desktop (please complete the following information):

AlexisDrogoul commented 2 years ago

Probably a rounding error. I guess serialisation is involved ? I noticed something a bit fishy in the converters regarding GamaPoint: the "reducer" writes the double values directly as Strings, which, depending on the client you're using, can be rounded differently. Could be interesting to see what is actually exchanged back and forth (i.e. the double values). Can you put it in a comment ?

agrignard commented 2 years ago

Ok I see.

So by printing the recieved message I am pretty surprise by the size of what is being received.

So here the message I receive in gama.client after sending this cmd

        cmd = {
            'type': 'output',
            'species': species1Name,
            'attributes': [attribute1Name],
            'socket_id': socket_id,
            'exp_id': exp_id,
            "callback": function (message) {
                if (typeof event.data == "object") {

                } else {
                    geojson = null;
                    geojson = JSON.parse(message);
                    console.log("geojson gama client ===> : " + message);
                    map.getSource('source1').setData(geojson);
                    canCallStaticLayer = true;

                }
                request = "";//IMPORTANT FLAG TO ACCOMPLISH CURRENT TRANSACTION
            }
        };

when having only one agent (of species people instanciated). It feels like there is way to many value of coordinates (I would assume one value per iteration)

geojson gama client ===> : {"type":"FeatureCollection","features":[{"type":"Feature","geometry":{"type":"Polygon","coordinates":[[[4.8780279159673,45.76809402179864,0.0],[4.878027303755771,45.768090520157585,0.0],[4.878025727192301,45.76808716938743,0.0],[4.878023246863485,45.76808409825639,0.0],[4.878019958087062,45.768081424786224,0.0],[4.878015987248898,45.768079251716856,0.0],[4.8780114869460265,45.76807766255805,0.0],[4.8780066301224565,45.7680767183802,0.0],[4.878001603423039,45.768076455467565,0.0],[4.8779966000208255,45.768076883923676,0.0],[4.877991812193582,45.768077987283235,0.0],[4.877987423934675,45.76807972314477,0.0],[4.877983603882339,45.76808202480019,0.0],[4.877980498839016,45.7680848037982,0.0],[4.877978228129822,45.76808795334354,0.0],[4.877976879016947,45.76809135240098,0.0],[4.877976503346204,45.768094870346715,0.0],[4.877977115554587,45.768098371988046,0.0],[4.877978692115454,45.768101722758864,0.0],[4.8779811724426,45.76810479389088,0.0],[4.8779844612185395,45.76810746736215,0.0],[4.877988432057486,45.76810964043262,0.0],[4.87799293236228,45.76811122959234,0.0],[4.8779977891886235,45.76811217377074,0.0],[4.878002815891247,45.768112436683566,0.0],[4.878007819296601,45.76811200822718,0.0],[4.878012607126451,45.76811090486694,0.0],[4.878016995387029,45.76810916900444,0.0],[4.8780208154398474,45.76810686734791,0.0],[4.878023920482391,45.76810408834881,0.0],[4.878026191189661,45.768100938802576,0.0],[4.878027540299757,45.76809753974454,0.0],[4.8780279159673,45.76809402179864,0.0]]]},"properties":{"type":"bike"},"id":"0"}]}

and here is what I receive from the VCity Client (the is a CRS difference but it seems pretty similar)

geojson VCity client-----> {"type":"FeatureCollection","features":[{"type":"Feature","geometry":{"type":"Polygon","coordinates":[[[1846068.0670822521,5175624.567827932,0.0],[1846068.028652813,5175624.177647287,0.0],[1846067.9148413173,5175623.802461066,0.0],[1846067.7300214767,5175623.456687465,0.0],[1846067.4812958145,5175623.153614369,0.0],[1846067.1782227182,5175622.904888706,0.0],[1846066.8324491172,5175622.720068866,0.0],[1846066.4572628962,5175622.606257371,0.0],[1846066.067082252,5175622.567827932,0.0],[1846065.676901608,5175622.606257371,0.0],[1846065.3017153875,5175622.720068866,0.0],[1846064.9559417863,5175622.904888706,0.0],[1846064.6528686895,5175623.153614369,0.0],[1846064.4041430275,5175623.456687465,0.0],[1846064.2193231871,5175623.802461066,0.0],[1846064.1055116917,5175624.177647287,0.0],[1846064.0670822521,5175624.567827932,0.0],[1846064.1055116917,5175624.958008575,0.0],[1846064.2193231871,5175625.333194796,0.0],[1846064.4041430275,5175625.678968397,0.0],[1846064.6528686895,5175625.982041494,0.0],[1846064.9559417863,5175626.230767156,0.0],[1846065.3017153875,5175626.415586996,0.0],[1846065.676901608,5175626.529398492,0.0],[1846066.067082252,5175626.567827932,0.0],[1846066.4572628962,5175626.529398492,0.0],[1846066.8324491172,5175626.415586996,0.0],[1846067.1782227182,5175626.230767156,0.0],[1846067.4812958145,5175625.982041494,0.0],[1846067.7300214767,5175625.678968397,0.0],[1846067.9148413173,5175625.333194796,0.0],[1846068.028652813,5175624.958008575,0.0],[1846068.0670822521,5175624.567827932,0.0]]]},"properties":{},"id":"0"}]}
agrignard commented 2 years ago

Probably a rounding error. I guess serialisation is involved ? I noticed something a bit fishy in the converters regarding GamaPoint: the "reducer" writes the double values directly as Strings, which, depending on the client you're using, can be rounded differently. Could be interesting to see what is actually exchanged back and forth (i.e. the double values). Can you put it in a comment ?

@AlexisDrogoul what do you mean by the 'reducer' ? If I want to make a test to send values as double and not as string, where should I change this in the code? Is it something I should do in SaveHelper.java?

hqnghi88 commented 2 years ago

Hi, Did you try with the newest binrary or from eclipse ? I cant reproduce this issue (hopefully related with what Alexis have recently change on serialization of objects)

agrignard commented 2 years ago

Ok so I just tried with the last versionof eclipse and I still get this error:

java.lang.IllegalArgumentException: Points of LinearRing do not form a closed linestring
    at org.locationtech.jts.geom.LinearRing.validateConstruction(LinearRing.java:93)
    at org.locationtech.jts.geom.LinearRing.<init>(LinearRing.java:88)
    at org.locationtech.jts.geom.GeometryFactory.createLinearRing(GeometryFactory.java:382)
    at org.locationtech.jts.geom.util.GeometryEditor$CoordinateSequenceOperation.edit(GeometryEditor.java:327)
    at org.locationtech.jts.geom.util.GeometryEditor.editInternal(GeometryEditor.java:160)
    at org.locationtech.jts.geom.util.GeometryEditor.edit(GeometryEditor.java:133)
    at org.locationtech.jts.geom.util.GeometryEditor.editPolygon(GeometryEditor.java:178)
    at org.locationtech.jts.geom.util.GeometryEditor.editInternal(GeometryEditor.java:152)
    at org.locationtech.jts.geom.util.GeometryEditor.edit(GeometryEditor.java:133)
    at org.locationtech.jts.geom.GeometryFactory.createGeometry(GeometryFactory.java:664)
    at msi.gama.metamodel.topology.projection.Projection.inverseTransform(Projection.java:144)
    at msi.gama.headless.common.SaveHelper.buildFeature(SaveHelper.java:223)
    at msi.gama.headless.common.SaveHelper.buildGeoJSon(SaveHelper.java:323)
    at msi.gama.headless.listener.GamaWebSocketServer.onMessage(GamaWebSocketServer.java:196)
    at org.java_websocket.server.WebSocketServer.onWebsocketMessage(WebSocketServer.java:712)
    at org.java_websocket.drafts.Draft_6455.processFrameText(Draft_6455.java:986)
    at org.java_websocket.drafts.Draft_6455.processFrame(Draft_6455.java:910)
    at org.java_websocket.WebSocketImpl.decodeFrames(WebSocketImpl.java:401)
    at org.java_websocket.WebSocketImpl.decode(WebSocketImpl.java:233)
    at org.java_websocket.server.WebSocketServer$WebSocketWorker.doDecode(WebSocketServer.java:1114)
    at org.java_websocket.server.WebSocketServer$WebSocketWorker.run(WebSocketServer.java:1086)

Note that it is happenign very very rarely using gama.client .

Howver it happens very quickly using another client (I have really no idea what can be the difference between the way the client and server are communicating as they are both using the same WebSocket architecture and cmd)

If you want to have a look here is the way to run the VCITY client

  1. Clone this repo https://github.com/VCityTeam/UD_ReAgent_ABM

  2. Set the rigth path to your model in UD-Viz-GCCV/src/bootstrap.js line 127

  3. in a terminal go to UD_ReAgent_ABM/UD-Viz-GCCV and do npm install then npm run debug

  4. Finally in a browser open loca](http://localhost:8000/)

  5. You should see few iteration running and then the error pops-up

hqnghi88 commented 2 years ago

Another client : you mean web browser?

agrignard commented 2 years ago

No same webbrowser but another client (the one developped intiallu for VCITY) https://github.com/VCityTeam/UD_ReAgent_ABM/blob/master/UD-Viz-GCCV/src/bootstrap.js

I cannot figure out what are the difference between the gama/client (using mapbox) and VCity (using itowns) in terms of code to communciate with GAMA they are very similar. With VCity i get the error after very few iteration always with gama.client i can get the error but it's very rare

hqnghi88 commented 2 years ago

It took me one full day to play with game of chance. Sometimes it occurs immediately, and maybe after 30 mins. I totally don't understand what happened, as the code base is totally the same with save statement, but if we save geojson in gama, it doesn't have this error.

hqnghi88 commented 2 years ago

Even with precision 1, shape of square, it still happens LINEARRING (166.5 181.2, 168.5 181.2, 168.5 179.2, 166.5 179.2, 166.5 181.2)

image
AlexisDrogoul commented 2 years ago

Very very strange. @ptaillandier .. what do you think of that ? How can it be with so simple geometries ?

ptaillandier commented 2 years ago

I was not able to reproduce this bug (GAMA git version, windows 10, chrome).

agrignard commented 2 years ago

Using gama.client the error happens but after a long time (around 30min and it's hard to reproduce) I guess this is what you have tested.

The initial issue I have is when I use another client (the one developped in Lyon (VCity , the issue is raised here https://github.com/VCityTeam/UD_ReAgent_ABM/issues/11 but I will reexplain here how ot reproduce it)

So the way to launch gama server is the same for the client size here is the way to reproduce the issue

  1. run Gama server from eclipse
  2. Clone this repo https://github.com/VCityTeam/UD_ReAgent_ABM
  3. Update the path to your local Gama model in /UD_ReAgent_ABM/UD-Viz-GCCV/src/bootstrap.jsby editing this variable modelPath(line 127) to point to this model UD_ReAgent_ABM/ReAgent/models/Gratte_Ciel_Basic.gaml
  4. from a terminal go to this repo /UD_ReAgent_ABM/UD-Viz-GCCV/
  5. run npm install
  6. Run npm run debug
  7. Open a browser on http://localhost:8000/
  8. After few iteration the error mention above about the non close linear ring appears pretty quickly

The question is now why it happens less on gama.client than UD_VIZ knowing that the code is really similar... @hqnghi88 is also investigating on this to fix it from the client side but would be great also to explore a fix from the Server side. It might be also very specific to this model but this error is never raised when I run the model on GAMA 1.8.2.

hqnghi88 commented 2 years ago

I tried to decrease the frequent of calculation by a thread-sleeping on both client server, but the message still occurs randomly. I think it is a bug of geotools somewhere in caching or cloning object as it crash by CoordSeqCloneOp.

image

However, the recall of computation immediately after try catch still work fine. So i pushed the workaround, @agrignard please test, it can play UDviz 2 hours until now. and seem ok to be 24h more

agrignard commented 2 years ago

Yes thank you @hqnghi88 I am testing this today and so far so good it seems pretty promising

I just see ones this message but it doesn't crash Points of LinearRing do not form a closed linestring

Not sure if we can consider this issue as closest or if we keep it open until we have a closer look at the potential issue with geotools.

lesquoyb commented 2 years ago

Closing as it seems that no other related problems have been found since the fix. Feel free to reopen if it's not the case