itinero / routing

The routing core of itinero.
Apache License 2.0
220 stars 70 forks source link

Question on Lua Not Equal and For, and UTurns #303

Open juliusfriedman opened 4 years ago

juliusfriedman commented 4 years ago

I have read https://www.moonsharp.org/moonluadifferences.html and cannot seem to find any reference to != vs ~=. See also https://stackoverflow.com/questions/34713203/what-does-lua-operator-mean/34713230

Can you point me to the documentation where you figured that out? Also would ~= be any faster / better?

juliusfriedman commented 4 years ago

Also wondering on the looping of the vehicle types, there seems to be a basic for loop in most of the profiles: https://github.com/itinero/routing/blob/develop/src/Itinero/Osm/Vehicles/car.lua#L117

I am wondering if it would be more efficient / better to use for ... in ... especially for cases when there are less than 10 keys.

for k, v in ipairs(vehicle_types) do

juliusfriedman commented 4 years ago

I have made the change locally to use for in and it seems to work just as well... I have not played with the != stuff yet.

I do however have one last question about the LUA while your here...

Looking at the profiles I can't really determine how turn cost is calculated via the profile.

I have a few scenarios where the route doubles back or it looks like U turn is performed, I can post a few samples if you need.

In these cases we know the train did not do a U turn so I am wondering how easy it would be to disable U turns via the profile...

I was thinking I could modify the is_oneway function to always return true? But I have not tested it yet because I been working on other things.

When you review the commits if you could address this question I would really appreciate it and then I will close this issue for you.

Thank you @xivk !

juliusfriedman commented 4 years ago

I attempted to modify the is_oneway and the results changed but were not better.

For the most part the routes generated are fine but in certain cases there are uturns(which are caused by missing points) which I am wondering how I can prevent, e.g. I want the route to take the the shortest or fastest but to not use any U Turns.

Here is a simple example: example.geojson.txt

The actual route is around Peterborough however due to U Turns the route appears to go through Leicester.

I am hoping that if this cannot be changed in the profile that you can share some sample code which uses the RouterSettings to prevent such.

I know if we had some additional points given along the Peterborough route considered the Router would find the correct route so I am wondering if there is not a better way to resolve the points than using TryResolveConnected, perhaps adding a synthetic point between the 2 points if necessary.

I am currently using the following code to build up the points given to the Router

//Try to resolve the point connected to the route going forward
                            var resolved = routing.TryResolveConnected(ProfileShortest, (float)ll.Latitude, (float)ll.Longitude, 2000, 1000, true);

                            //If there was an error then use the normal resolve
                            if (resolved.IsError)
                            {
                                resolved = router.TryResolve(ProfileShortest, (float)ll.Latitude, (float)ll.Longitude, 1000);

                                //If still an error than discard it
                                if (resolved.IsError) continue;
                            }

But according to this

Another thing we did is add support for U-turn prevention. While this is something we already have in the Itinero routing engine itself, there is also a need to implement this at the level of the optimization code.

There should be a way without adding any points and just specifying something although I cannot find where to specify it.

I really need to know how to achieve the prevention of u-turn using only Itinero either by a profile hack or otherwise.

Thank you!

juliusfriedman commented 4 years ago

@xivk just wondering if you will have any time to help me out on this as well as review the PR's I made for you:

https://github.com/itinero/routing/pull/302

Which requires

https://github.com/itinero/reminiscence/pull/23

After both are applied the issues with using Parallel are gone and I don't seem to have inconsistent results anymore when using a contracted vs non contracted database.

It also seems to address a a few other small things such as GetDistanceInMeters (https://github.com/itinero/routing/issues/279) now being correct as well as the issue when getting different weights were being obtained from TryCalculateWeight (https://github.com/itinero/routing/issues/293)

I have also address https://github.com/itinero/routing/issues/306 but I don't want to commit further until you have verified my changes.

IE there might be one other place where CalculateManyToMany needs to be replaced with the ManyToManyAlgorithmn.

Thank you!

juliusfriedman commented 4 years ago

I have experimented with TryCalulcate doing something like this:

var route = Router.TryCalculate(profile ?? TrainProfileFastest, routerPoints, float.MaxValue, (float?[])null, cancellationToken);

But I get:

System.NotSupportedException: Contraction-based many-to-many calculates are not supported in the given router db for the given profile.

Yet my RouterDb is not contracted. Looking at the source it implies I NEED to have a contracted RouterDb for this to work. is that true?

I am going to try on a contracted version but it significantly increased the file size because I need the functionality for at least 2 * 2 profiles on a 400MB non-contracted db which is resulting in a something like 1.2 - 1.6 GB RouterDb.

When I attempt to contract now I get an exception:

Edge detected that's too long in ...

It seems that was related to my changed code for https://github.com/itinero/routing/issues/306 in MaxDistanceSplitter, I found it by exploring the call hierarchy for MaxEdgeDistance.

I will wait for your comments before I make any further changes to the PR. But I have confirmed that once the <= is changed back to < in MaxDistanceSplitter that I can again perform contraction. I am waiting for that to complete now so I can again test my TryCalculate with turnPenalty specified.

After contracting I was able to generate routes using the code I posted above, they mostly look better but I am going to have to analyze them with my team on Monday.

Please do let me know if you find out whats up with MaxDistanceSplitter I will hold off on updating the PR again until you can review.