-
Notifications
You must be signed in to change notification settings - Fork 237
feat: implement multi transport types for the same dataset #2579
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
base: main
Are you sure you want to change the base?
Changes from all commits
21b46e0
862e09d
9173ac6
0f53d1a
829157e
772ab5e
2a66083
a5f4a27
fa23335
13fea0e
192f7da
bbf4d33
b2a8e83
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -147,6 +147,97 @@ class TimeslotHolidayEnricher : SolverModelEnricher<Timetable> { | |
| -- | ||
| ==== | ||
|
|
||
| [#transportTypes] | ||
| == Transport types | ||
|
|
||
| Models whose solution implements `LocationsAwareSolverModel` are enriched by the built-in `TravelTimeMatrixEnricher`, | ||
| which fetches travel time and distance matrices from the map service and stores them on each `Location`. | ||
|
|
||
| Which routing profile those matrices describe is controlled by the transport type: | ||
|
|
||
| [cols="1,3"] | ||
| |=== | ||
| | Value | Meaning | ||
|
|
||
| | `car` | ||
| | The default. Distances follow the road network as driven by a car. | ||
|
|
||
| | `bicycle` | ||
| | Distances follow the cycling network. | ||
|
|
||
| | `foot` | ||
| | Distances follow the walking network. | ||
|
|
||
| | `auto-select` | ||
| | Not a routing profile. The transport types are taken from the dataset instead; see <<autoSelect>>. | ||
| |=== | ||
|
|
||
| Set it with the `timefold.platform.map-service.transport-type` property. | ||
| A deployment may additionally be restricted to a subset of transport types with | ||
| `timefold.platform.map-service.allowed-transport-types`, a comma-separated list; when it is not set, every transport | ||
| type is allowed. | ||
|
|
||
| A dataset that uses a transport type other than the configured one is rejected before any map service call is made, | ||
| because a deployment only has the map for the profile it is configured with. | ||
|
|
||
| [#readingMatrices] | ||
| === Reading travel times and distances | ||
|
|
||
| `Location` exposes an overload of each lookup that takes the transport type. | ||
| Use it whenever your model distinguishes between transport types: | ||
|
|
||
| [source,java,options="nowrap"] | ||
| ---- | ||
| TravelTime travelTime = origin.getTravelTimeTo(destination, TransportType.BICYCLE); | ||
| TravelDistance distance = origin.getDistanceTo(destination, TransportType.BICYCLE); | ||
|
|
||
| // The traffic-aware overloads take the departure time as well. | ||
| TravelTime atNoon = origin.getTravelTimeTo(destination, departureTime, TransportType.BICYCLE); | ||
| ---- | ||
|
|
||
| The overloads without a transport type keep working and resolve to the primary transport type of the deployment, | ||
| which is defined in <<autoSelect>>. | ||
|
|
||
| If your model stores matrices itself rather than relying on the enricher, the setters take the transport type too: | ||
|
|
||
| [source,java,options="nowrap"] | ||
| ---- | ||
| location.setTravelTimeMatrix(TransportType.BICYCLE, travelTimeMatrix); | ||
| location.setDistanceMatrix(TransportType.BICYCLE, distanceMatrix); | ||
|
Comment on lines
+201
to
+206
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Let's not expose this as an API. We should take care of the matrices, not the user. |
||
| ---- | ||
|
|
||
| [#autoSelect] | ||
| === Routing one dataset with several transport types | ||
|
|
||
| With `timefold.platform.map-service.transport-type=auto-select`, the transport types are not fixed by configuration. | ||
| The enricher asks the solution which ones the dataset actually uses, by overriding `getTransportTypes()`: | ||
|
|
||
| [source,java,options="nowrap"] | ||
| ---- | ||
| @Override | ||
| public List<TransportType> getTransportTypes() { | ||
| return vehicles.stream() | ||
| .map(Vehicle::getTransportType) | ||
| .filter(Objects::nonNull) | ||
| .distinct() | ||
| .toList(); | ||
| } | ||
| ---- | ||
|
|
||
| Each of those transport types is validated against the allowed transport types and then fetched in its own map service | ||
| request, so a dataset using two transport types results in two requests and two sets of matrices per `Location`. | ||
| A dataset that uses a transport type the deployment is not allowed to route with is rejected before any request is made. | ||
| When the solution returns an empty list, the default transport type is used. | ||
|
|
||
| One of the transport types is the *primary* one: the configured one when it is fixed, otherwise `car` if the dataset | ||
| uses it, and the first one in declaration order otherwise. The primary transport type is special in two ways: | ||
|
|
||
| * The model-level map metadata — the locations that are not in the map, and the resolved map region — comes from its | ||
| request only. | ||
| * Its matrices also back the lookups that do not name a transport type, so those keep answering on a deployment | ||
| configured for a single non-default transport type. | ||
|
Comment on lines
+227
to
+238
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why should the user care? This text explains how we implement the transport types, but that's the complexity we are trying to remove from the user. |
||
|
|
||
|
|
||
| [#enrichmentDirector] | ||
| == Controlling enrichment order | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -50,6 +50,24 @@ public class Location { | |
| @JsonIgnore | ||
| private ToIntFunction<OffsetDateTime> timeframeIndexResolver; | ||
|
|
||
| // Number of transport modes; used to size the per-mode arrays below. | ||
| private static final int MODE_COUNT = TransportType.values().length; | ||
|
|
||
| // Non-default transport types only. The default mode ({@link TransportType#CAR}) continues to use the scalar/ | ||
| // timeframe fields above so its lookups keep the IndexableDistanceMatrix index-cache fast path. | ||
| // Indexed by {@link TransportType#ordinal()} for fast hot-path lookups; allocated lazily on first non-default set. | ||
| @JsonIgnore | ||
| private DistanceMatrix[] travelTimeMatrixByMode; | ||
|
|
||
| @JsonIgnore | ||
| private DistanceMatrix[] distanceMatrixByMode; | ||
|
|
||
| @JsonIgnore | ||
| private DistanceMatrix[][] travelTimesByTimeframeByMode; | ||
|
|
||
| @JsonIgnore | ||
| private DistanceMatrix[][] distancesByTimeframeByMode; | ||
|
|
||
| public Location() { | ||
| } | ||
|
|
||
|
|
@@ -120,6 +138,54 @@ public void setDistanceMatrices(DistanceMatrix[] distancesByTimeframe, | |
| } | ||
| } | ||
|
|
||
| public void setTravelTimeMatrix(TransportType transportType, DistanceMatrix travelTimeMatrix) { | ||
| if (isDefaultMode(transportType)) { | ||
| setTravelTimeMatrix(travelTimeMatrix); | ||
| return; | ||
| } | ||
| if (travelTimeMatrixByMode == null) { | ||
| travelTimeMatrixByMode = new DistanceMatrix[MODE_COUNT]; | ||
| } | ||
| travelTimeMatrixByMode[transportType.ordinal()] = travelTimeMatrix; | ||
| } | ||
|
|
||
| public void setDistanceMatrix(TransportType transportType, DistanceMatrix distanceMatrix) { | ||
| if (isDefaultMode(transportType)) { | ||
| setDistanceMatrix(distanceMatrix); | ||
| return; | ||
| } | ||
| if (distanceMatrixByMode == null) { | ||
| distanceMatrixByMode = new DistanceMatrix[MODE_COUNT]; | ||
| } | ||
| distanceMatrixByMode[transportType.ordinal()] = distanceMatrix; | ||
| } | ||
|
|
||
| public void setTravelTimeMatrices(TransportType transportType, DistanceMatrix[] travelTimesByTimeframe, | ||
| ToIntFunction<OffsetDateTime> indexResolver) { | ||
| if (isDefaultMode(transportType)) { | ||
| setTravelTimeMatrices(travelTimesByTimeframe, indexResolver); | ||
| return; | ||
| } | ||
| if (travelTimesByTimeframeByMode == null) { | ||
| travelTimesByTimeframeByMode = new DistanceMatrix[MODE_COUNT][]; | ||
| } | ||
| travelTimesByTimeframeByMode[transportType.ordinal()] = travelTimesByTimeframe; | ||
| this.timeframeIndexResolver = indexResolver; | ||
| } | ||
|
|
||
| public void setDistanceMatrices(TransportType transportType, DistanceMatrix[] distancesByTimeframe, | ||
| ToIntFunction<OffsetDateTime> indexResolver) { | ||
| if (isDefaultMode(transportType)) { | ||
| setDistanceMatrices(distancesByTimeframe, indexResolver); | ||
| return; | ||
| } | ||
| if (distancesByTimeframeByMode == null) { | ||
| distancesByTimeframeByMode = new DistanceMatrix[MODE_COUNT][]; | ||
| } | ||
| distancesByTimeframeByMode[transportType.ordinal()] = distancesByTimeframe; | ||
| this.timeframeIndexResolver = indexResolver; | ||
| } | ||
|
|
||
| /** | ||
| * Returns the travel time for a route between this location and the given location. | ||
| * | ||
|
|
@@ -226,6 +292,69 @@ public TravelDistance getDistanceTo(Location location, OffsetDateTime departureT | |
| return TravelDistance.of(distance); | ||
| } | ||
|
|
||
| /** | ||
| * Returns the travel time for a route between this location and the given location using the given transport type. | ||
| * | ||
| * @param location the location representing the route destination | ||
| * @param transportType the routing profile to use; {@code null} is treated as {@link TransportType#CAR} | ||
| * @return {@link TravelTime} instance representing the travel time in seconds. | ||
| * @throws IllegalArgumentException When the resolved matrix does not include both locations. | ||
| * @throws IllegalStateException When no travel time matrix is configured for the given transport type. | ||
| */ | ||
| public TravelTime getTravelTimeTo(Location location, TransportType transportType) { | ||
| DistanceMatrix matrix = travelTimeMatrixForMode(transportType); | ||
| return TravelTime.of(lookup(matrix, location, transportType, "travel time", null)); | ||
| } | ||
|
|
||
| /** | ||
| * Returns the travel time for a route between this location and the given location at the given departure time, | ||
| * using the given transport type. | ||
| * | ||
| * @param location the location representing the route destination | ||
| * @param departureTime the instant used to select the traffic timeframe matrix | ||
| * @param transportType the routing profile to use; {@code null} is treated as {@link TransportType#CAR} | ||
| * @return {@link TravelTime} instance representing the travel time in seconds. | ||
| * @throws IllegalArgumentException When the resolved matrix does not include both locations, or the resolver | ||
| * returns an out-of-bounds index. | ||
| * @throws IllegalStateException When no travel time matrix is configured for the given transport type. | ||
| */ | ||
| public TravelTime getTravelTimeTo(Location location, OffsetDateTime departureTime, TransportType transportType) { | ||
| DistanceMatrix matrix = travelTimeMatrixForMode(transportType, departureTime); | ||
| return TravelTime.of(lookup(matrix, location, transportType, "travel time", departureTime)); | ||
| } | ||
|
|
||
| /** | ||
| * Returns the travel distance for a route between this location and the given location using the given transport | ||
| * type. | ||
| * | ||
| * @param location the location representing the route destination | ||
| * @param transportType the routing profile to use; {@code null} is treated as {@link TransportType#CAR} | ||
| * @return {@link TravelDistance} instance representing the travel distance in meters. | ||
| * @throws IllegalArgumentException When the resolved matrix does not include both locations. | ||
| * @throws IllegalStateException When no distance matrix is configured for the given transport type. | ||
| */ | ||
| public TravelDistance getDistanceTo(Location location, TransportType transportType) { | ||
| var matrix = distanceMatrixForMode(transportType); | ||
| return TravelDistance.of(lookup(matrix, location, transportType, "distance", null)); | ||
| } | ||
|
|
||
| /** | ||
| * Returns the travel distance for a route between this location and the given location at the given departure | ||
| * time, using the given transport type. | ||
| * | ||
| * @param location the location representing the route destination | ||
| * @param departureTime the instant used to select the traffic timeframe matrix | ||
| * @param transportType the routing profile to use; {@code null} is treated as {@link TransportType#CAR} | ||
| * @return {@link TravelDistance} instance representing the travel distance in meters. | ||
| * @throws IllegalArgumentException When the resolved matrix does not include both locations, or the resolver | ||
| * returns an out-of-bounds index. | ||
| * @throws IllegalStateException When no distance matrix is configured for the given transport type. | ||
| */ | ||
| public TravelDistance getDistanceTo(Location location, OffsetDateTime departureTime, TransportType transportType) { | ||
| var matrix = distanceMatrixForMode(transportType, departureTime); | ||
| return TravelDistance.of(lookup(matrix, location, transportType, "distance", departureTime)); | ||
| } | ||
|
|
||
| public short getIndex(DistanceMatrix matrix) { | ||
| if (matrix == travelTimeMatrix) { | ||
| return travelTimeMatrixIndex; | ||
|
|
@@ -280,6 +409,72 @@ private DistanceMatrix resolveTimeframeMatrix(DistanceMatrix[] matrices, OffsetD | |
| return matrix; | ||
| } | ||
|
|
||
| private static boolean isDefaultMode(TransportType transportType) { | ||
| return transportType == null || TransportType.CAR == transportType; | ||
| } | ||
|
|
||
| private DistanceMatrix travelTimeMatrixForMode(TransportType transportType) { | ||
| if (isDefaultMode(transportType)) { | ||
| return travelTimeMatrix; | ||
| } | ||
| return travelTimeMatrixByMode == null ? null : travelTimeMatrixByMode[transportType.ordinal()]; | ||
| } | ||
|
|
||
| private DistanceMatrix travelTimeMatrixForMode(TransportType transportType, OffsetDateTime departureTime) { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Since we are adding more logic to resolve the matrix on the hot path, do we have any benchmarks showing the impact on move evaluation speed for multiple vs. a single transport type? |
||
| if (isDefaultMode(transportType)) { | ||
| return hasTimeframeMatrices(travelTimesByTimeframe) | ||
| ? resolveTimeframeMatrix(travelTimesByTimeframe, departureTime, "travel time") | ||
| : travelTimeMatrix; | ||
| } | ||
| DistanceMatrix[] byTimeframe = travelTimesByTimeframeByMode == null | ||
| ? null | ||
| : travelTimesByTimeframeByMode[transportType.ordinal()]; | ||
| if (byTimeframe != null && timeframeIndexResolver != null) { | ||
| return resolveTimeframeMatrix(byTimeframe, departureTime, "travel time"); | ||
| } | ||
| return travelTimeMatrixByMode == null ? null : travelTimeMatrixByMode[transportType.ordinal()]; | ||
| } | ||
|
|
||
| private DistanceMatrix distanceMatrixForMode(TransportType transportType) { | ||
| if (isDefaultMode(transportType)) { | ||
| return distanceMatrix; | ||
| } | ||
| return distanceMatrixByMode == null ? null : distanceMatrixByMode[transportType.ordinal()]; | ||
| } | ||
|
|
||
| private DistanceMatrix distanceMatrixForMode(TransportType transportType, OffsetDateTime departureTime) { | ||
| if (isDefaultMode(transportType)) { | ||
| return hasTimeframeMatrices(distancesByTimeframe) | ||
| ? resolveTimeframeMatrix(distancesByTimeframe, departureTime, "distance") | ||
| : distanceMatrix; | ||
| } | ||
| DistanceMatrix[] byTimeframe = distancesByTimeframeByMode == null | ||
| ? null | ||
| : distancesByTimeframeByMode[transportType.ordinal()]; | ||
| if (byTimeframe != null && timeframeIndexResolver != null) { | ||
| return resolveTimeframeMatrix(byTimeframe, departureTime, "distance"); | ||
| } | ||
| return distanceMatrixByMode == null ? null : distanceMatrixByMode[transportType.ordinal()]; | ||
| } | ||
|
Comment on lines
+423
to
+458
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. According to SonarCloud, these methods that resolve the distance matrix are not covered by tests. We likely miss more tests that would indirectly trigger them. |
||
|
|
||
| private long lookup(DistanceMatrix matrix, Location to, TransportType transportType, String what, | ||
| OffsetDateTime departureTime) { | ||
| TransportType resolvedMode = transportType == null ? TransportType.CAR : transportType; | ||
| if (matrix == null) { | ||
| throw new IllegalStateException( | ||
| "No %s matrix configured for a location (%s) and transport type (%s).".formatted(what, this, | ||
| resolvedMode)); | ||
| } | ||
| long value = matrix.get(this, to); | ||
| if (value == -1) { | ||
| String at = departureTime == null ? "" : " at (%s)".formatted(departureTime); | ||
| throw new IllegalArgumentException(("No %s information found for a route from (%s) to (%s) for transport " | ||
| + "type (%s)%s. Are both locations in the configured map and in the location set (if used)?") | ||
| .formatted(what, this, to, resolvedMode, at)); | ||
| } | ||
| return value; | ||
| } | ||
|
|
||
| private void updateIndex(DistanceMatrix distanceMatrix) { | ||
| if (distanceMatrix instanceof IndexableDistanceMatrix indexableDistanceMatrix) { | ||
| indexableDistanceMatrix.updateCachedIndex(this); | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,55 @@ | ||
| package ai.timefold.solver.service.maps.api.model; | ||
|
|
||
| import java.util.List; | ||
| import java.util.Objects; | ||
|
|
||
| import org.eclipse.microprofile.openapi.annotations.media.Schema; | ||
|
|
||
| import com.fasterxml.jackson.annotation.JsonCreator; | ||
| import com.fasterxml.jackson.annotation.JsonValue; | ||
|
|
||
| @Schema(description = "The type of transport used (car, bicycle, ... ).") | ||
| public enum TransportType { | ||
|
|
||
| CAR("car"), | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we need the lower-case value? If we drop it, we don't need to override the enum methods nor creating the constructor. |
||
| BICYCLE("bicycle"), | ||
| FOOT("foot"), | ||
| AUTO_SELECT("auto-select"); | ||
|
|
||
| /** | ||
| * Every transport type that maps to an actual routing profile, i.e. all but {@link #AUTO_SELECT}. | ||
| */ | ||
| public static final List<TransportType> ROUTING_PROFILES = List.of(CAR, BICYCLE, FOOT); | ||
|
|
||
| private final String value; | ||
|
|
||
| TransportType(String value) { | ||
| Objects.requireNonNull(value, "TransportType value must not be null."); | ||
| value = value.trim().toLowerCase(); | ||
| if (value.isEmpty()) { | ||
| throw new IllegalArgumentException("TransportType value must not be blank."); | ||
| } | ||
| this.value = value; | ||
| } | ||
|
|
||
| @JsonCreator | ||
| public static TransportType of(String value) { | ||
| Objects.requireNonNull(value, "TransportType value must not be null."); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Compared to the constructor, we don't check if the But see my proposal about simplifying the enum, which would leave only this method and eliminate the constructor. |
||
| // Both the wire value ("auto-select") and the enum name ("AUTO_SELECT") are accepted. | ||
| return TransportType.valueOf(value.trim().toUpperCase().replace('-', '_')); | ||
| } | ||
|
|
||
| @JsonValue | ||
| public String value() { | ||
| return value; | ||
| } | ||
|
|
||
| public boolean isAutoSelect() { | ||
| return this == AUTO_SELECT; | ||
| } | ||
|
|
||
| @Override | ||
| public String toString() { | ||
| return value; | ||
| } | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Who restricts that?
I assume this comes as a platform config, in which case the model developer cannot much influence that if they don't run the model locally.