riebl / artery

OMNeT++ V2X simulation framework for ETSI ITS-G5
GNU General Public License v2.0
203 stars 129 forks source link

preselection polygon is invalid: Geometry has spikes #261

Closed Lagertonne closed 1 year ago

Lagertonne commented 1 year ago

Hi,

I'm using artery in combination with a 4x4 grid generated by netgenerate, and some trips generated with randomTrips.py (both provided by SUMO). All vehicles are equipped with a SeeThroughSensor and a RadarSensor.

But when I enable the SeeThroughSensor, I'm getting the following error message after some time: artery_error_spikes

When disabling the SeeThroughSensor, the error message disappears.

I'm using the "stock" SeeThroughSensor with only the following line in the omnetpp.ini: *.node[*].environmentModel.SeeThrough.fovRange = 50m

I already narrowed it down to this line in the code throwing the error, but I'm not sure what to do with this information: https://github.com/riebl/artery/blob/master/src/artery/envmod/GlobalEnvironmentModel.cc#L336

I'm using artery with Xubuntu 22.04, omnetpp 5.6.2 and SUMO 1.12.0

awillecke commented 1 year ago

Hey,

that's a strange behavior. However, it is hard to reproduce this error without having your specific scenario. You could push the scenario to a branch in a fork of Artery.

Also, knowing the exact polygon that fails to validate could be useful. Could apply this commit (https://github.com/awillecke/artery/commit/7a7c88b58103f8739d7109cc5ad526eaaea773d5) to your code and post the output of the error (preferably as text)?

Best regards

Lagertonne commented 1 year ago

Hi,

thanks for your response!

I recreated a failing scenario here: https://github.com/Lagertonne/artery/tree/buggy_geometry_spikes It fails at around 197 simulation seconds with the following message:

preselection polygon is invalid: Geometry has spikes -- in module (artery::SeeThroughSensor) World.node[30].environmentModel.SeeThrough (id=2219), at t=197.5s, event #3966425

I've run the simulation 3 times and the error message was always the same. The scenario was built as a Release, so that should speed things up a little bit. The error message also appeared when built in debug mode.

Sadly, I could't apply your patch. The error message was the following:

/home/lagertonne/artery/src/artery/envmod/GlobalEnvironmentModel.cc: In member function ‘std::vector<std::shared_ptr<artery::EnvironmentModelObject> > artery::GlobalEnvironmentModel::preselectObjects(const string&, const std::vector<artery::Position>&)’:
/home/lagertonne/artery/src/artery/envmod/GlobalEnvironmentModel.cc:338:117: error: ‘to_wkt’ is not a member of ‘boost::geometry’
  338 |         throw omnetpp::cRuntimeError("preselection polygon is invalid:%s (%s)", error_msg.c_str(), boost::geometry::to_wkt(area).c_str());
      |                                                                                                                     ^~~~~~
gmake[2]: *** [src/artery/envmod/CMakeFiles/envmod.dir/build.make:90: src/artery/envmod/CMakeFiles/envmod.dir/GlobalEnvironmentModel.cc.o] Error 1
gmake[1]: *** [CMakeFiles/Makefile2:1113: src/artery/envmod/CMakeFiles/envmod.dir/all] Error 2
gmake: *** [Makefile:146: all] Error 2

I tried to apply the header files mentioned here (https://www.boost.org/doc/libs/1_80_0/libs/geometry/doc/html/geometry/reference/io/wkt/to_wkt.html) to fix this but did not have any luck with that.

EDIT: Small addition, I'm using the "envmod" configuration from the omnetpp.ini to reproduce this.

awillecke commented 1 year ago

I was able to reproduce your issue with your branch.

The polygon of interest is defined as POLYGON((51.95 383.16,38.4606 361.072,15.734 348.687,-10.14 349.325,-32.2286 362.814,-44.6132 385.541,-43.9753 411.415,-30.4858 433.504,-7.75928 445.888,18.1148 445.25,40.2034 431.761,52.588 409.034,51.95 383.16)) and looks something like this:

buggy_geometry_spikes

This seems fine to me.

When looking at high precision Positions, however, the first point of the polygon is 51.950041451175473, 383.16019200817749 and the last point is 51.950041451175508, 383.16019200817755 which are very close, but not identical. I guess this might be some kind of floating point issue?

hagau commented 1 year ago

@awillecke I've encountered this issue before too, but couldn't reproduce it on another machine. I suspect an issue in boost, but stepping through the library code has not resulted in anything. As you mentioned, it's a floating point issue; somehow, when constructing the sensor cone polygon, the point rotation accumulates errors which results in the last point not being exactly equal to the first.

As a quick fix, I copied the coordinates of the first point to the last, see https://github.com/hagau/artery/commit/32257f3722d6868f75baf7c1003c55b7d3239ac9.

awillecke commented 1 year ago

Hey @hagau, thanks for helping out and providing a fix.

I like this approach, because it evades the issue of floating point precision (at least in this case). Otherwise, a discussion on required precision would be necessary, I guess.

hagau commented 1 year ago

Well, this is (most likely) an issue with a specific version of an underlying library, possibly also compiler flags. In a separate test, the point rotation implementation in boost.geometry seems to work perfectly well for 1000+ points. The code in SensorConfiguration::createSensorArc is correct, so I'm not sure we should actually make changes there in reaction to a ephemeral bug.

But maybe explicitly copying the first point is better than just rotating since boost requires that for the polygons used in queries anyway :slightly_smiling_face:. In which case I need to rewrite the patch to eliminate the superfluous rotation for the last point.

I just tried to reproduce the error on the machine I had encountered it before (Ubuntu 22.04.1, SUMO 1.13, OMNet++ 5.6.2, Clang 13.0, Boost 1.74). I can't seem to trigger the bug anymore. It's still the same boost version, but boost.geometry is a header-only library, so it will get compiled with the same flags as artery.

riebl commented 1 year ago

Thanks for reporting this issue and all your analyses! In fact, I think we can omit the last geometry point for circles entirely because our geometry definition is "open". Please have a look at my "fix_sensor_geometry" branch. At a first glance, the result looks promising. However, I had no time to thoroughly test it, so I am looking forward to your feedback before merging this fix!

Lagertonne commented 1 year ago

As a quick fix, I copied the coordinates of the first point to the last, see hagau@32257f3.

Thanks for all the answers, this solves my problem for now :)

riebl commented 1 year ago

The _fix_sensorgeometry patch is included in master now.