itinero / routing

The routing core of itinero.
Apache License 2.0
221 stars 69 forks source link

[Exception] Cannot read elements with an id outside of the accessor range #154

Closed goersen closed 6 years ago

goersen commented 6 years ago

Setup:

1. OSM-Data of germany (http://download.geofabrik.de/europe/germany-latest.osm.pbf)
2. Create a routerdb
3. Add contracted graph for Itinero.OSM.Vehicles.Vehicle.Car.Fastest() 
4. Save and reuse that routerdb for everything
5. Running in a .NET Standard Library 2.0 (i.e. compatible with normal .Net Framework)
6. Tested on the last couple of prereleases (29, 30, 31)

Data:

Additional Info:

xivk commented 6 years ago
  • I regularly get this error from router.Calculate(profile, points); :(.
  • On the same route it doesn't occur every single time, but it does occur every 2nd or 3rd time.
  • Calculating this route, then another, then this again almost certainly produces the error
  • Above route is only an example. Longer routes with more points seem to be more problematic than routes with less points.
  • The longer the distance (i.e. traveling back and forth between cities) also increases error-rate.

Ok, I'll try to reproduce this and get back to you. This obviously shouldn't be happening. Thanks for properly reporting all the details BTW.

  • "Fun" fact: using router.CalculateTSP(profile, coords, 0, 0); will not produce the error. (You have to switch out the first point because the resolve of the first point will fail with default radius) ((Btw... can we get a resolve-radius for CalculateTSP? Would be soooo awesome!))

It's not exposed by default but you can do this by implementing another version of the CalculateTSP extension (but perhaps it should be there by default):

https://github.com/itinero/optimization/blob/develop/src/Itinero.Optimization/RouterExtensions.cs#L61

You can add a MassResolvingAlgorithm to the WeightMatrixAlgorithm:

https://github.com/itinero/routing/blob/develop/src/Itinero/Algorithms/Matrices/WeightMatrixAlgorithm.cs#L51

And here you can customize all this:

https://github.com/itinero/routing/blob/a5a58056abe167cb8cb767d0aada41edfbd14de8/src/Itinero/Algorithms/Search/MassResolvingAlgorithm.cs#L41

Perhaps I should consider exposing all this is a better way, feel free to add an issue to the optimization repo.

goersen commented 6 years ago

Thanks @xivk for the super quick response!

I'm a little bit embarrassed but I just realized what's probably the cause of the issue :/.

While calculating that route we also calculate the distance from your current position to all the stops. Everything is done on background-threads as to not block the UI. But this means that on longer routes chances are that more than one route is calculated at a time. When disabling the "distance to each stop" calculation, everything works fine.

I assume calculating multiple routes on the same routerdb at the same time, is what breaks it. We use a new Router each time but only the one RouterDb.

So this might actually not be a bug or at least is not easily fixed. I can understand if running multiple calculations at once is not supported.

P.S. Also big big thanks for taking the time to give some pointers on the MassResolver for TSP! You are awesome! :)

xivk commented 6 years ago

Ah, thanks again for the feedback, I was wondering if that would be the issue, multithreading, but didn't suggest it because this is supposed to be supported and tested pretty well.

It's best-practice to keep the router around and reuse that. The router caches some data, like precalculated speeds for each used profile, it's better for performance to pass that around instead of the routerdb. Otherwise it will recalculate speeds for every profile on each request. It may also cause the synchronization issues:

alecava58 commented 6 years ago

@goersen in case it would interesting for you, I had the same problem and I solved with the following snippet where I set the parameter radiusInMeter as resolve radius and searchDistanceInMeter for mass resolver: InstanceProfile is the routing profile (car, pedestrian etc.)

       `
        IList<Coordinate> coo = new List<Coordinate>();
        for (var i = 0; i < latParam.Length; i++)
            coo.Add(new Coordinate((float)latParam[i], (float)lonParam[i]));

        // points resolution function with check connectivity
        Func<Itinero.Data.Network.RoutingEdge, int, bool> edgeFunction = (edge, i) =>
        {
            // create a temp resolved point in the middle of this edge.
            var tempRouterPoint = new RouterPoint(0, 0, edge.Id, ushort.MaxValue / 2);
            var connectivityResult = router.TryCheckConnectivity(InstanceProfile, tempRouterPoint, radiusInMeter, null);
            if (connectivityResult.IsError)
            { // if there is an error checking connectivity, choose not report it, just don't choose this point.
                return false;
            }
            return connectivityResult.Value;
        };

        // mass resolution function with weight calculation (distance)
        // the standard function doesn't use TryCheckConnectivity
        MassResolvingAlgorithm massResolver = new MassResolvingAlgorithm(router,
            new Profile[] { InstanceProfile },
            coo.ToArray(),
            edgeFunction,
            searchDistanceInMeter);

        // TSP calculation
        TSPRouter routerTSP;
        if (param.optimizeNoReturn.HasValue && param.optimizeNoReturn.Value == true)
            routerTSP = new TSPRouter(new WeightMatrixAlgorithm(router, InstanceProfile, massResolver), 0);
        else
            routerTSP = new TSPRouter(new WeightMatrixAlgorithm(router, InstanceProfile, massResolver), 0, coo.Count - 1);

        routerTSP.Run();
        Log.WriteToLog(LogLevel.Information, string.Format("End OptimizeTSP: has succeeded = {0}", routerTSP.HasSucceeded));
        return routerTSP.Tour;`
goersen commented 6 years ago

@xivk Thanks!!

Passing the Router instead of RouterDb fixed it. Tested with bigger routes, more routes and more switching around. No Errors.

:-)

goersen commented 6 years ago

@alecava58

Thanks for the snippet! We'll look into it :)

xivk commented 6 years ago

Closing this, opened an issue on the docs repo for what's left todo here.

Hessi9 commented 5 years ago

I got the same error at parallel execution. It was caused from the creating a WeightHandler that isn't thread save. When I created the WeightHandler once in advance and reused it for all requests it worked well. To create it I used the DefaultWeightHandlerCached(routerDb) extention function of the used Profile instance.

drspam1991 commented 5 years ago

@goersen

Can you explain your solution ("Passing the Router instead of RouterDb") with sample code?

YuriiNskyi commented 4 years ago

@goersen joining to drspam1991`s question, could you explain your solution? @xivk has the same problem. I am already using router as singleton, seems that it doesn't prevent the error.