Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(routing): upgrade to GraphHopper 8.0 #363

Merged
merged 22 commits into from
Mar 21, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
354edb0
feat(routing): upgrade GraphHopper to 8.0, several refactorings
kschrab Oct 31, 2023
e29df56
fix(routing): fixed speed conversion
kschrab Oct 31, 2023
2ca4bee
fix(routing): allow configuring alternative route parameters
kschrab Nov 2, 2023
b587e47
test: reverted accidental commit
kschrab Nov 2, 2023
1d19f29
feat(routing): reactivate cleanup, move weighting instantiation to we…
kschrab Nov 2, 2023
1a16932
fix(routing): resolved compilation error
kschrab Nov 2, 2023
1b00f5d
fix(routing): removed double configuration of alternative route params
kschrab Nov 2, 2023
ce8cc96
fix(routing): resolve spotbugs warnings
kschrab Nov 2, 2023
f242643
refactor(routing): get rid of ExtendedGraphHopper, load grap in Graph…
kschrab Nov 3, 2023
d09edd4
legal: approve third party content
kschrab Nov 3, 2023
f519de7
clean: re-added valuable comment about virtual edges
kschrab Nov 3, 2023
fc608a0
clean: removed unused GraphLoader interface, moved VehicleEncoding to…
kschrab Nov 3, 2023
5714d91
clean: make alternative routing parameters accessible to alter via tr…
kschrab Nov 3, 2023
2a29681
Merge remote-tracking branch 'origin/main' into 716-graphhopper-8
kschrab Jan 29, 2024
4b297ea
fix(routing): revive changes overridden by merge
kschrab Jan 29, 2024
ed25671
fix(mosaic-routing): calcEdgeWeight should return values as seconds
kschrab Jan 31, 2024
e8223d5
fix(mosaic-routing): round max speed before
kschrab Jan 31, 2024
c3819c0
test(mosaic-routing): use 60 km/h as speed for test graph to restore …
kschrab Jan 31, 2024
5a685f4
clean(mosaic-routing): minor cleanup after self-review
kschrab Jan 31, 2024
1fcfaf0
Merge remote-tracking branch 'origin/main' into 716-graphhopper-8
kschrab Mar 20, 2024
7128d75
fix(routing): fixed waytype encoding and addressed further cleanups a…
kschrab Mar 21, 2024
dc8deb0
fix(routing): suppress spotbugs warning
kschrab Mar 21, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
70 changes: 39 additions & 31 deletions NOTICE-THIRD-PARTY.md
Original file line number Diff line number Diff line change
Expand Up @@ -93,22 +93,22 @@ FindBugs-jsr305 (3.0.2)
* Source: https://code.google.com/p/jsr-305/


GraphHopper API (0.13.0)

* License: Apache License, Version 2.0
* Maven artifact: `com.graphhopper:graphhopper-api:0.13.0`
* Project: https://www.graphhopper.com/graphhopper-api
* Source: https://github.com/graphhopper/graphhopper/graphhopper-api


GraphHopper Core (0.13.0)
GraphHopper Core (8.0)

* License: The Apache Software License, Version 2.0
* Maven artifact: `com.graphhopper:graphhopper-core:0.13.0`
* Maven artifact: `com.graphhopper:graphhopper-core:8.0`
* Project: https://www.graphhopper.com/graphhopper-core
* Source: https://github.com/graphhopper/graphhopper/graphhopper-core


GraphHopper Web API (8.0)

* License: Apache License, Version 2.0
* Maven artifact: `com.graphhopper:graphhopper-web-api:8.0`
* Project: https://www.graphhopper.com/graphhopper-web-api
* Source: https://github.com/graphhopper/graphhopper/graphhopper-web-api
schwepmo marked this conversation as resolved.
Show resolved Hide resolved


Gson (2.10.1)

* License: Apache-2.0
Expand All @@ -133,46 +133,46 @@ HPPC Collections (0.8.1)
* Source: https://github.com/carrotsearch/hppc/hppc


Jackson module: JAXB Annotations (2.9.9)
Jackson-annotations (2.15.0)

* License: The Apache Software License, Version 2.0
* Maven artifact: `com.fasterxml.jackson.module:jackson-module-jaxb-annotations:2.9.9`
* Project: https://github.com/FasterXML/jackson-modules-base
* Source: https://github.com/FasterXML/jackson-modules-base/jackson-module-jaxb-annotations


Jackson-annotations (2.9.0)

* License: The Apache Software License, Version 2.0
* Maven artifact: `com.fasterxml.jackson.core:jackson-annotations:2.9.0`
* Project: http://github.com/FasterXML/jackson
* Maven artifact: `com.fasterxml.jackson.core:jackson-annotations:2.15.0`
* Project: https://github.com/FasterXML/jackson
* Source: https://github.com/FasterXML/jackson-annotations


Jackson-core (2.9.9)
Jackson-core (2.15.0)

* License: The Apache Software License, Version 2.0
* Maven artifact: `com.fasterxml.jackson.core:jackson-core:2.9.9`
* Maven artifact: `com.fasterxml.jackson.core:jackson-core:2.15.0`
* Project: https://github.com/FasterXML/jackson-core
* Source: https://github.com/FasterXML/jackson-core


jackson-databind (2.9.9)
jackson-databind (2.15.0)

* License: The Apache Software License, Version 2.0
* Maven artifact: `com.fasterxml.jackson.core:jackson-databind:2.9.9`
* Project: http://github.com/FasterXML/jackson
* Maven artifact: `com.fasterxml.jackson.core:jackson-databind:2.15.0`
* Project: https://github.com/FasterXML/jackson
* Source: https://github.com/FasterXML/jackson-databind


Jackson-dataformat-XML (2.9.9)
Jackson-dataformat-XML (2.15.0)

* License: The Apache Software License, Version 2.0
* Maven artifact: `com.fasterxml.jackson.dataformat:jackson-dataformat-xml:2.9.9`
* Project: http://wiki.fasterxml.com/JacksonExtensionXmlDataBinding
* Maven artifact: `com.fasterxml.jackson.dataformat:jackson-dataformat-xml:2.15.0`
* Project: https://github.com/FasterXML/jackson-dataformat-xml
* Source: https://github.com/FasterXML/jackson-dataformat-xml


Jackson-datatype-jts (2.14)

* License: The Apache Software License, Version 2.0
* Maven artifact: `com.graphhopper.external:jackson-datatype-jts:2.14`
* Project: https://github.com/bedatadriven/jackson-datatype-jts/
* Source: scm:git:https://github.com/bedatadriven/jackson-datatype-jts


Janino (2.7.5)

* License: New BSD License
Expand Down Expand Up @@ -237,6 +237,14 @@ org.leadpony.justify (1.1.0)
* Source: https://github.com/leadpony/justify


org.locationtech.jts:jts-core (1.19.0)

* License: Eclipse Distribution License - v 1.0, Eclipse Public License, Version 2.0
* Maven artifact: `org.locationtech.jts:jts-core:1.19.0`
* Project: https://www.locationtech.org/projects/technology.jts/jts-modules/jts-core
* Source: https://github.com/locationtech/jts/jts-modules/jts-core


Protocol Buffers [Core] (3.8.0)

* License: 3-Clause BSD License
Expand Down Expand Up @@ -269,10 +277,10 @@ Stax2 API (4.2.1)
* Source: https://github.com/FasterXML/stax2-api


Woodstox (5.1.0)
Woodstox (6.5.1)

* License: The Apache License, Version 2.0
* Maven artifact: `com.fasterxml.woodstox:woodstox-core:5.1.0`
* Maven artifact: `com.fasterxml.woodstox:woodstox-core:6.5.1`
* Project: https://github.com/FasterXML/woodstox
* Source: https://github.com/FasterXML/woodstox

Expand Down
4 changes: 3 additions & 1 deletion bundle/src/assembly/mosaic-bundle.xml
Original file line number Diff line number Diff line change
Expand Up @@ -154,9 +154,11 @@
<!-- [END] XML object binding -->

<!-- [START] GraphHopper dependencies -->
<include>com.graphhopper:graphhopper-api</include>
<include>com.graphhopper:graphhopper-web-api</include>
<include>com.graphhopper:graphhopper-core</include>
<include>com.graphhopper.external:jackson-datatype-jts</include>
<include>com.carrotsearch:hppc</include>
<include>org.locationtech.jts:jts-core</include>
<!-- [END] GraphHopper dependencies -->

<include>com.jcraft:jsch</include>
Expand Down
10 changes: 0 additions & 10 deletions lib/mosaic-routing/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -32,16 +32,6 @@
<dependency>
<groupId>com.graphhopper</groupId>
<artifactId>graphhopper-core</artifactId>
<exclusions>
<exclusion>
<groupId>org.locationtech.jts</groupId>
<artifactId>jts-core</artifactId>
</exclusion>
<exclusion>
<groupId>org.apache.xmlgraphics</groupId>
<artifactId>xmlgraphics-commons</artifactId>
</exclusion>
</exclusions>
</dependency>
<dependency>
<groupId>com.google.guava</groupId>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,25 +23,94 @@
*/
public class CandidateRoute {

/**
* The list of connections forming this route.
*/
private final List<String> connectionIds;

/**
* The length in meters of this route.
*/
private final double length;
schwepmo marked this conversation as resolved.
Show resolved Hide resolved

/**
* The approximated driving time in seconds on this route.
*/
private final double time;
schwepmo marked this conversation as resolved.
Show resolved Hide resolved

/**
* The distance in meters from the start node of the first connection in the connectionIds list, to
* the point the source query was issued.
*/
private final double offsetFromSource;
schwepmo marked this conversation as resolved.
Show resolved Hide resolved

/**
* The distance in meters from the point the target query was issued, until the end node of the
* final connection in the connectionIds list.
*/
private final double offsetToTarget;

/**
* @param connectionIds the list of connection id this route persists of.
* @param length the length in m of this route.
* @param time the approximate travel time in seconds of this route.
*/
public CandidateRoute(List<String> connectionIds, double length, double time) {
schwepmo marked this conversation as resolved.
Show resolved Hide resolved
this(connectionIds, length, time, 0, 0);
}

/**
*
* @param connectionIds the list of connection id this route persists of.
* @param length the length in m of this route.
* @param time the approximate travel time in seconds of this route.
* @param offsetFromSource the distance in meters from the start node of the first connection in the connectionIds list, to
* the point the source query was issued.
* @param offsetToTarget the distance in meters from the point the target query was issued, until the end node of the
* final connection in the connectionIds list.
*/
public CandidateRoute(List<String> connectionIds, double length, double time, double offsetFromSource, double offsetToTarget) {
schwepmo marked this conversation as resolved.
Show resolved Hide resolved
this.connectionIds = connectionIds;
this.length = length;
this.time = time;
this.offsetFromSource = offsetFromSource;
this.offsetToTarget = offsetToTarget;
}

/**
* @return the list of connection ids forming this route.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think these type of javadocs leads to spotbugs warnings, as the actual documentation is missing.
Personally, I think the documentation for getters should either be really verbose (like the member variable) or none.

Copy link
Contributor Author

@kschrab kschrab Mar 21, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In this particular case I'd need the documentation at the getters, since the user wants to know what the offset is doing. However, for getters I find it personally more useful to fill @return and leave the description blank, as this would be the same text. I'd rather adjust our checkstyle...

*/
public List<String> getConnectionIds() {
return connectionIds;
}

/**
* @return the length in meters of this route.
*/
public double getLength() {
return length;
}

/**
* @return the approximated driving time in seconds on this route.
*/
public double getTime() {
return time;
}

/**
* @return the distance in meters from the start node of the first connection in the connectionIds list, to
* the point the source query was issued.
*/
public double getOffsetFromSource() {
return offsetFromSource;
}

/**
* @return the distance in meters from the point the target query was issued, until
* the end node of the final connection in the connectionIds list.
*/
public double getOffsetToTarget() {
return offsetToTarget;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -86,8 +86,7 @@ public void initialize(final CRouting configuration, final File baseDirectory) t
}

//creates an implementation of IRoutingGraph according to the configuration
this.routing = new GraphHopperRouting()
.loadGraphFromDatabase(scenarioDatabase);
this.routing = new GraphHopperRouting(scenarioDatabase);

this.routeManager = new RouteManager(this.scenarioDatabase);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,12 @@
import org.eclipse.mosaic.lib.routing.EdgeProperties;
import org.eclipse.mosaic.lib.routing.RoutingCostFunction;
import org.eclipse.mosaic.lib.routing.graphhopper.util.GraphhopperToDatabaseMapper;
import org.eclipse.mosaic.lib.routing.graphhopper.util.VehicleEncoding;
import org.eclipse.mosaic.lib.routing.graphhopper.util.WayTypeEncoder;

import com.google.common.collect.Iterables;
import com.graphhopper.routing.util.FlagEncoder;
import com.graphhopper.util.EdgeIteratorState;
import com.graphhopper.util.FetchMode;
import org.apache.commons.lang3.Validate;

import java.util.Optional;
Expand All @@ -32,16 +34,18 @@
* Provides properties from the current {@link EdgeIteratorState} or
* its belonging {@link Connection} to be used by an {@link RoutingCostFunction}.
*/
class GraphHopperEdgeProperties implements EdgeProperties {
public class GraphHopperEdgeProperties implements EdgeProperties {

private final FlagEncoder flagEncoder;
private final VehicleEncoding encoding;
private final WayTypeEncoder wayTypeEncoder;
private final GraphhopperToDatabaseMapper graphMapper;

private EdgeIteratorState currentEdgeIterator;
private boolean reverseRequests;

GraphHopperEdgeProperties(FlagEncoder encoder, GraphhopperToDatabaseMapper graphMapper) {
this.flagEncoder = encoder;
GraphHopperEdgeProperties(VehicleEncoding encoding, WayTypeEncoder wayTypeEncoder, GraphhopperToDatabaseMapper graphMapper) {
this.encoding = encoding;
this.wayTypeEncoder = wayTypeEncoder;
this.graphMapper = graphMapper;
}

Expand All @@ -54,9 +58,9 @@ void setCurrentEdgeIterator(EdgeIteratorState currentEdgeIterator, boolean rever
@Override
public double getSpeed() {
Validate.notNull(currentEdgeIterator, "Edge iterator is null");
return (reverseRequests
? currentEdgeIterator.getReverse(flagEncoder.getAverageSpeedEnc())
: currentEdgeIterator.get(flagEncoder.getAverageSpeedEnc())) / 3.6d;
return reverseRequests
? currentEdgeIterator.getReverse(encoding.speed()) / 3.6
: currentEdgeIterator.get(encoding.speed()) / 3.6;
}

@Override
Expand All @@ -69,8 +73,8 @@ public double getLength() {
public Iterable<GeoPoint> getGeometry() {
Validate.notNull(currentEdgeIterator, "Edge iterator is null");
return Iterables.transform(
currentEdgeIterator.fetchWayGeometry(3), // 3 = fetch all pillar nodes inclusive the base and adjacent tower node
ghPoint3D -> GeoPoint.latLon(ghPoint3D.getLat(), ghPoint3D.getLon(), ghPoint3D.getElevation())
currentEdgeIterator.fetchWayGeometry(FetchMode.ALL), // fetches all pillar nodes inclusive the base and adjacent tower node
ghPoint3D -> GeoPoint.latLon(ghPoint3D.getLat(), ghPoint3D.getLon(), ghPoint3D.getEle())
);
}

Expand All @@ -81,7 +85,11 @@ public String getConnectionId() {

@Override
public String getWayType() {
return getConnection().map(con -> con.getWay().getType()).orElse(null);
return WayTypeEncoder.decode(getWayTypeEncoded());
}

public int getWayTypeEncoded() {
return currentEdgeIterator.get(wayTypeEncoder);
}

private Optional<Connection> getConnection() {
Expand Down
Loading
Loading