Allow setting new value for OSRM snapping radius

[SimonBradley1993]

We have an issue where Vroom is assigning jobs to vehicles where there is no valid route, due to OSRM snapping from the nearest location to the job location. I see that Vroom sets quite a high radius value of 35000m. It'd be really useful if we can adjust this value with config to allow us to set this much lower.

[jcoupey]

The rationale for setting a very high value was to not change too much previous behavior when introducing the filtering, while erroring on typical mistakes (e.g. lon,lat mixup). But I agree we should probably use a smaller default value.

As for adjusting the value to your needs, that is definitely possible:

vroom/src/structures/typedefs.h

Line 59 in 004faa4

const std::string DEFAULT_OSRM_SNAPPING_RADIUS = "35000";

[SimonBradley1993]

Hi Julien,

I'm trying now to experiment with having this value set to a different one. I don't seem to have the rights to push a branch to the vroom repo to then pull that branch with Vroom docker.

Are you able to give me permissions to push a new branch?

Thanks

[SimonBradley1993]

Forget that, I've realised I need to use my fork to do this

[SimonBradley1993]

Hi Julien,

I've been testing some changes to Vroom in relation to this issue. I've come across a check that happens for null values when parsing the OSRM response. This is the function that is doing this check check_unfound in wrapper.h.

After making my OSRM snapping radius change, we get expected null values coming back from OSRM. This is due to some of our vehicles being located on islands, meaning that they won't have any route to jobs or other vehicles on the mainland. When running these requests through Vroom we then get errors coming back from the check_unfound function that stops the process.

The expected behaviour for us would be that vehicles that don't have a valid route to jobs would not be assigned those jobs, instead of the whole process being stopped. When I remove this check we get this behaviour and can add more jobs to islands where there are vehicles located.

Would removing this check be something that you would be able to / want to do in Vroom?

[SimonBradley1993]

Hi Julien,

I've opened a couple of PR's that would allow us to configure the snapping radius for OSRM :

#1332
VROOM-Project/vroom-express#115

I appreciate I've probably missed some things, especially with the Vroom PR. I'm happy for you to modify it as you like, if you want this feature in.

I've tested this locally with vroom-express and it works as expected, adjusting the radius query parameter values sent to OSRM.

[jcoupey]

Thanks for sharing your changes! My main concern here is that I don't think it's wise to add a high-level vroom flag just for the sake of a "small" routing parameter (and only targeting one routing engine so far). Do you really need to set different values on a per-request basis? Looks to me that once one has found a good default value then having it hardcoded is not really a problem?

[jcoupey]

The expected behaviour for us would be that vehicles that don't have a valid route to jobs would not be assigned those jobs, instead of the whole process being stopped.

Stopping the whole process in case of matrix inconsistency may sound a bit drastic, but on the other end if we don't we'll likely experience situations with weird unexpected solutions that are very hard to explain and debug.

When I remove this check we get this behaviour and can add more jobs to islands where there are vehicles located.

Off the top of my head I have no idea how those null values are treated down the line when removing the check, and how that impacts job to vehicle insertion evaluations, so you're in unknown territory here. ;-)

More to the point: if you have big islands generating isolated connected components from a routing perspective I would argue that it does not make sense to solve the problem as a whole, you could just solve the independent sub-problems, each with consistent matrices.

[SimonBradley1993]

Thanks for taking a look Julien. You're right, this probably isn't really needed as a new flag for all of Vroom. We've currently got a version of Vroom deployed that is using my fork branch.

The islands that we're coming across are very small, with a single vehicle being moved to it for a short period of time to complete a very low number of jobs - which is part of a much bigger area with other vehicles not on the island. So isolating these tiny islands would probably be possible but I don't think the business would want to do that with there being many, many islands.

I'll get these draft PR's closed and just run with our forked version.

Thanks!