Use PrepareCHGraph adapter for CH preparation

[easbar]

This PR adds a new class PrepareCHGraph (+ corresponding PrepareCHEdgeExplorer+Iterator) that basically wraps CHGraph to provide a smaller interface used by PrepareContractionHierarchies and the NodeContractors.

These are the most important changes:

• There is PrepareCHIterator.getWeight(), which either returns the shortcut weight or calculates the weight of an original edge. Previously we exposed the EdgeIteratorState interface of the iterator, and passed it into PrepareWeighting again to make this distinction. Now that the Weighting is available inside the iterator we can achieve the same without exposing all the EdgeIteratorState methods. Maybe this approach has benefits in other places as well like checking for edgeWeight==infinity instead of using accessEnc inside the edge iterator).

• There is a new class NodeBasedWitnessPathSearcher, which at the moment is a copy of DijkstraOneToMany minus some methods that are not needed for CH preparation like calcPaths() etc. This is partly necessary, because PrepareCHGraph is not a Graph and partly useful, because we can adjust it to the CH preparation without having to worry about other uses of DijkstraOneToMany. It sure means there is some kind of code duplication, but I think this is not so much of a problem here/its better than making the code overly generic in this case (imo, happy to hear other opinions).

• PrepareCHGraph has methods like createIn/OutEdgeExplorer, so we no longer need to pass the edge filter/access enc when creating the explorer.

Why is this useful ? Its maybe not so apparent here already, but it takes some 'load' off CHGraph, because CHGraph is used in multiple ways that can be optimized for their specific use. Currently we use it to:

1. run CH algorithms on it
2. find shortcuts during CH preparation
3. add shortcuts to it (build it)
4. store it on disk, load it from memory etc.

PrepareCHGraph does 2) and 3) for now, even though this will probably be split further into two separate parts soon. Most importantly PrepareCHGraph is not (compared to the previous CHGraph) concerned with 1) and 4) anymore.

So far there is no new functionality, but this PR is part of a bigger refactoring: #1780. Its even very likely that the code changed here will change again very soon. I still thought it might be useful to merge this already to prevent a giant pull request at the end of this refactoring. What do you think ?

[karussell]

I really like that the accessEnc usage and FlagEncoder usage is reduced. (For #1776 we need this)

But isn't it actually just a bit moved around? E.g. it is now used directly in PrepareCHEdgeIterator.java instead of the EdgeFilter ... hmmh, no in PrepareCHGraph still the EdgeFilter is used. And why are there isForward & isBackward methods directly in the iterator? (in the future I would prefer to rely only on the Weighting for this)

So I'm a bit unsure about the goal of this refactoring - can you explain? Or is this mainly driven by #1818 and then the changes become more clear?

[easbar]

But isn't it actually just a bit moved around?

Yes, its just moved around. I would argue it is now easier to use the weighting to determine the fwd/bwd access, because we have it right there in the iterator. But this is not very crucial here (it would not yet justify all this refactoring).

And why are there isForward & isBackward methods directly in the iterator? (in the future I would prefer to rely only on the Weighting for this)

I think its only needed for some heuristic in edge-based CH preparation, but its probably possible to remove it. But implementing this via the weighting would be very easy, because the iterator now has access to the weighting.

in PrepareCHGraph still the EdgeFilter is used.

Yes. This is because at the moment PrepareCHGraph simply relies on CHGraph (its just a wrapper around CHGraph plus the Weighting), but this is more or less temporary. I am almost certain that it is very inefficient to use CHGraph during the preparation, because the number of shortcuts increases during the preparation (there are more and more shortcuts, even though we filter them out). This is what I meant with splitting PrepareCHGraph into 2) and 3). Sorry its probably hard to follow.

So I'm a bit unsure about the goal of this refactoring - can you explain?

Yes I understand, I am sure its hard to see the benefits at the moment :) The main goal is separating the concerns of CHGraph, because it does (at least) the four things I mentioned above and they all can be optimized in a different way:

1. run CH algorithms on it: Here the CH graph is read only and we do not need to add shortcuts anymore, so its most important to use a memory layout that allows for fast edge iteration. Also its important to use little memory to allow loading large graphs / multiple profiles. Also when we are considering flexible CH these algorithms should know very little about the CH graph implementation. This is the main reason for Decouple CHGraph from (Base)Graph functionality #1780: Right know the CHGraph interface is way too broad.

2. find shortcuts during CH preparation: To do this its most beneficial to entirely remove edges at lower levels, because they are no longer considered for witness paths. This requires us to efficiently remove edges from the CH graph (instead of just filtering them out as we currently do)

3. add shortcuts to it (build it): this also happens during the CH preparation, but the CH preparation being built does not necessarily have to be the same graph as the one used in 2).

4. store it on disk, load it from memory etc.: This does not require a specific data structure, but its important to be able to store/load the graph we obtain in 3) while for 1) these methods are entirely irrelevant.

The migration path I see is to change all the code that uses CHGraph such that it only uses the parts of it it is interested in. For example the NodeContractors only care about what is PrepareCHGraph here. When this is done it should be easier to change the single parts into something that is optimized for the specific use case. At the beginning I would like to simply delegate to the existing CHGraph implementation (thats what I did here for the PrepareCHGraph).

[easbar]

And why are there isForward & isBackward methods directly in the iterator? (in the future I would prefer to rely only on the Weighting for this)

Ok, but how ? I am thinking the easiest way is what I did here:

// the graph is weighted: it knows the weighting. 
MyGraph g = new MyGraph(baseGraph, weighting);
// when the explorer is created it also gets a reference to the weighting internally
MyIter iter = g.createOutEdgeExplorer().setBaseNode();
// in next() the iterator is able to use the weighting and check if the access is blocked/weight is infinite etc.
while (iter.next()) {
   // this is an outgoing edge
}

I am not saying we should put the weighting into the base graph, but to me it seems to make a lot of sense to pass a 'weighted graph' (a wrapper of the base graph + weighting) to the code that traverses the graph (at least as long as it is code that should run on a weighted graph, such as the shortest path algorithms).

The isForward()/isBackward() methods are more or less convenience (and optimization): We can use an all-edge iterator and check the fwd/bwd-ness ourselves (instead of using a separate in/out edge iterator). Internally it can work just the same way: The weighting is used to determine whether the weight is infinite/access is blocked.

[karussell]

Thanks, I do not understand everything but already more :) ... also the flexible CH is something I have not thought of.

I am thinking the easiest way is what I did here

For BaseGraph I think we can have it very simple:

while(edgeIter.next()) {
     // in a separate PR we move the accessEnc check into the weighting:
     double weight = weighting.calcWeight(edgeIter, reverse, prevEdge);
     if(Double.isInfinite(weight)) continue; // block access

     // do something with edgeIter
}

For PrepareCHGraph we could put this logic into the edgeIter.next call but having two separate calls (e.g. isForward and weighting.calcWeight) will make refactoring #1776 harder IMO. So I would like to say conceptionally goodbye to access restrictions and FlagEncoders and only use Weighting when we need to know if the weight is 0 or 10 or whatever or infinity (==access blocked). We need to make the hard constraint "access restrictions" a soft constraint (example: "bike profile should be able to traverse one-way roads in reverse direction with 6km/h" or police_car should be able to use car but make access restrictions "soft" - i.e. it can go everywhere but carefully.)

[easbar]

Ok, so your biggest concern is calling calcWeight multiple times ? I think we could even improve (speed) performance here already by storing the weight instead of the weighting within the PrepareCHEdgeIterator edges. This way calcWeight would be only called twice per edge instead of the probably millions of times during all these local dijkstra searches during CH preparation. Sure this would increase memory usage, but only during import and only proportional to the number of base graph edges. If that is not an option at the moment I will find a way to call it only once and/or get rid of isForward/Backward, no problem.

[easbar]

We need to make the hard constraint "access restrictions" a soft constraint (

Ok I understand. That makes sense and also your 'very' simple method works, because in many cases we do not even need to check for weight==infinite, because when the weight is infinite the shortest path tree is simply not being further explored in this direction. If this helps I think it would not be a big problem to get rid of createIn/OutEdgeExplorer in PrepareCHEdgeExplorer (This reflects the 'hard' constraint as we do it at the moment, but probably makes less sense with the refactoring you have in mind).

[karussell]

Thanks for the changes - feel free to merge this :)

Ok, so your biggest concern is calling calcWeight multiple times ?

Yes, but also conceptionally asking for "accessible?" and then again does not seem to be "good". But if it is cached it wouldn't matter much, yes.

Sure this would increase memory usage, but only during import and only proportional to the number of base graph edges

Yes, interesting idea to do this only for the preparation.

because when the weight is infinite the shortest path tree is simply not being further explored in this direction

In case of "infinite" it is indeed called just once, but if access=true (should be most of the time) it is called twice (?)

[easbar]

I think the/your biggest concern is that with createIn/OutEdge we are enforcing the notion of incoming/outgoing edges in code (normally a good thing in my opinion) but for the configurable weighting you/we want to do the opposite and soften this constraint. So thanks for the reminder, I will consider this from now.

Yes, interesting idea to do this only for the preparation.

I have this in mind, but would rather postpone this a bit. Do you (already) have weightings in mind that might be expensive to evaluate ? Especially when running some user defined scripts etc. to calculate the weight this might become important.

In case of "infinite" it is indeed called just once, but if access=true (should be most of the time) it is called twice (?)

Ah I thought like this its not called twice:

while(edgeIter.next()) {
     // in a separate PR we move the accessEnc check into the weighting:
     double weight = weighting.calcWeight(edgeIter, reverse, prevEdge);
     if(Double.isInfinite(weight)) continue; // block access

     // use weight here
}

[karussell]

If this helps I think it would not be a big problem to get rid of createIn/OutEdgeExplorer in PrepareCHEdgeExplorer (

Yes, I think we need to refactor this, but in a later PR and likely with all the other usages of EdgeFilter and EdgeExplorer.

but for the configurable weighting you/we want to do the opposite and soften this constraint

👍

Do you (already) have weightings in mind that might be expensive to evaluate ?

Probably we should indeed benchmark before optimizing :)

Ah I thought like this its not called twice:

I meant the way it was before with isForward. If isForward is true, then we have called calcWeigh and then again for evaluating the actual weighting.