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

Add 'transferParametersForMode' build config field #6215

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
35 commits
Select commit Hold shift + click to select a range
36336bf
Add ability to create special transfer requests for cars and bikes wi…
VillePihlava Nov 1, 2024
f1380e3
Add tests for 'carsAllowedStopMaxTransferDurationsForMode' build conf…
VillePihlava Nov 1, 2024
2175e64
Move if statement outside of for loop.
VillePihlava Nov 1, 2024
6418e26
Merge branch 'dev-2.x' of github.com:opentripplanner/OpenTripPlanner …
VillePihlava Nov 5, 2024
e1e9bbb
Fixes based on review comments.
VillePihlava Dec 3, 2024
89e617f
Add TransferParameters to build config.
VillePihlava Dec 3, 2024
a1e0807
Remove mentions of carsAllowedStopMaxTransferDurationsForMode and fix…
VillePihlava Dec 3, 2024
21bb307
Rename variables and small improvements.
VillePihlava Dec 5, 2024
573078a
Change test.
VillePihlava Dec 5, 2024
3fd5bf5
Add documentation for build config fields.
VillePihlava Dec 5, 2024
35c98bc
Add logging for transfers.
VillePihlava Dec 5, 2024
3a34aa4
Merge branch 'split-transfers-by-mode-pathtransfer-mode' into car-tra…
VillePihlava Dec 5, 2024
97238a3
Fix merge issues.
VillePihlava Dec 5, 2024
41a0674
Add tests and small improvements.
VillePihlava Dec 10, 2024
1a3f846
Merge branch 'dev-2.x' of github.com:opentripplanner/OpenTripPlanner …
VillePihlava Dec 16, 2024
2e048f9
Refactor transfer parameter parsing.
VillePihlava Dec 16, 2024
c90c3ec
Remove duplicate test.
VillePihlava Dec 16, 2024
30cc925
Simplify implementation by removing changes to StreetNearbyStopFinder.
VillePihlava Dec 17, 2024
2e57c2f
Remove unnecessary parameter from function call.
VillePihlava Dec 17, 2024
f2296cd
Add formatting for JSON in documentation.
VillePihlava Jan 7, 2025
0ac172e
Rename variables.
VillePihlava Jan 7, 2025
bff77a5
Refactor transfer generation.
VillePihlava Jan 7, 2025
9f5d63c
Rename transfers to transferParameters in the build config and throw …
VillePihlava Jan 7, 2025
4f84804
Change wording of documentation.
VillePihlava Jan 7, 2025
4d599a2
Add comments and refactor transfer generation.
VillePihlava Jan 7, 2025
2eb0912
Rename transfers to transferParameters in documentation.
VillePihlava Jan 7, 2025
57e03b4
Change default values from Duration.ZERO to null.
VillePihlava Jan 14, 2025
7de4a39
Add documentation.
VillePihlava Jan 14, 2025
8e87974
Rename transferParameters to transferParametersForMode.
VillePihlava Jan 14, 2025
d0fbae9
Change documentation for maxTransferDuration.
VillePihlava Jan 14, 2025
f385061
Add documentation to standalone build-config.json and NodeAdapterHelper.
VillePihlava Jan 14, 2025
afa1b16
Use method to get field instead of the field directly.
VillePihlava Jan 16, 2025
c649ab7
Change comment style.
VillePihlava Jan 16, 2025
91782ef
Split transfer generation into methods.
VillePihlava Jan 16, 2025
186d20c
Merge branch 'dev-2.x' of github.com:opentripplanner/OpenTripPlanner …
VillePihlava Jan 21, 2025
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

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
package org.opentripplanner.graph_builder.module;

import java.time.Duration;
import org.opentripplanner.utils.tostring.ToStringBuilder;

/**
* Mode-specific parameters for transfers.
*/
public record TransferParameters(
VillePihlava marked this conversation as resolved.
Show resolved Hide resolved
Duration maxTransferDuration,
Duration carsAllowedStopMaxTransferDuration,
boolean disableDefaultTransfers
) {
public static final Duration DEFAULT_MAX_TRANSFER_DURATION = null;
public static final Duration DEFAULT_CARS_ALLOWED_STOP_MAX_TRANSFER_DURATION = null;
public static final boolean DEFAULT_DISABLE_DEFAULT_TRANSFERS = false;

TransferParameters(Builder builder) {
this(
builder.maxTransferDuration,
builder.carsAllowedStopMaxTransferDuration,
builder.disableDefaultTransfers
);
}

public String toString() {
return ToStringBuilder
.of(getClass())
.addDuration("maxTransferDuration", maxTransferDuration)
.addDuration("carsAllowedStopMaxTransferDuration", carsAllowedStopMaxTransferDuration)
.addBool("disableDefaultTransfers", disableDefaultTransfers)
.toString();
}

public static class Builder {

private Duration maxTransferDuration;
private Duration carsAllowedStopMaxTransferDuration;
private boolean disableDefaultTransfers;

public Builder() {
this.maxTransferDuration = DEFAULT_MAX_TRANSFER_DURATION;
this.carsAllowedStopMaxTransferDuration = DEFAULT_CARS_ALLOWED_STOP_MAX_TRANSFER_DURATION;
this.disableDefaultTransfers = DEFAULT_DISABLE_DEFAULT_TRANSFERS;
}

public Builder withMaxTransferDuration(Duration maxTransferDuration) {
this.maxTransferDuration = maxTransferDuration;
return this;
}

public Builder withCarsAllowedStopMaxTransferDuration(
Duration carsAllowedStopMaxTransferDuration
) {
this.carsAllowedStopMaxTransferDuration = carsAllowedStopMaxTransferDuration;
return this;
}

public Builder withDisableDefaultTransfers(boolean disableDefaultTransfers) {
this.disableDefaultTransfers = disableDefaultTransfers;
return this;
}

public TransferParameters build() {
return new TransferParameters(this);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -259,7 +259,8 @@ static DirectTransferGenerator provideDirectTransferGenerator(
timetableRepository,
issueStore,
config.maxTransferDuration,
config.transferRequests
config.transferRequests,
config.transferParametersForMode
);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
import java.time.LocalDate;
import java.time.ZoneId;
import java.util.List;
import java.util.Map;
import java.util.Set;
import java.util.regex.Pattern;
import javax.annotation.Nullable;
Expand All @@ -23,6 +24,7 @@
import org.opentripplanner.ext.emissions.EmissionsConfig;
import org.opentripplanner.ext.fares.FaresConfiguration;
import org.opentripplanner.framework.geometry.CompactElevationProfile;
import org.opentripplanner.graph_builder.module.TransferParameters;
import org.opentripplanner.graph_builder.module.ned.parameter.DemExtractParameters;
import org.opentripplanner.graph_builder.module.ned.parameter.DemExtractParametersList;
import org.opentripplanner.graph_builder.module.osm.parameters.OsmExtractParameters;
Expand All @@ -32,13 +34,16 @@
import org.opentripplanner.model.calendar.ServiceDateInterval;
import org.opentripplanner.netex.config.NetexFeedParameters;
import org.opentripplanner.routing.api.request.RouteRequest;
import org.opentripplanner.routing.api.request.StreetMode;
import org.opentripplanner.routing.api.request.framework.DurationForEnum;
import org.opentripplanner.routing.fares.FareServiceFactory;
import org.opentripplanner.standalone.config.buildconfig.DemConfig;
import org.opentripplanner.standalone.config.buildconfig.GtfsConfig;
import org.opentripplanner.standalone.config.buildconfig.IslandPruningConfig;
import org.opentripplanner.standalone.config.buildconfig.NetexConfig;
import org.opentripplanner.standalone.config.buildconfig.OsmConfig;
import org.opentripplanner.standalone.config.buildconfig.S3BucketConfig;
import org.opentripplanner.standalone.config.buildconfig.TransferConfig;
import org.opentripplanner.standalone.config.buildconfig.TransferRequestConfig;
import org.opentripplanner.standalone.config.buildconfig.TransitFeedConfig;
import org.opentripplanner.standalone.config.buildconfig.TransitFeeds;
Expand Down Expand Up @@ -151,6 +156,7 @@ public class BuildConfig implements OtpDataStoreConfig {
public final IslandPruningConfig islandPruning;

public final Duration maxTransferDuration;
public final Map<StreetMode, TransferParameters> transferParametersForMode;
public final NetexFeedParameters netexDefaults;
public final GtfsFeedParameters gtfsDefaults;

Expand Down Expand Up @@ -284,9 +290,10 @@ When set to true (it is false by default), the elevation module will include the
.of("maxTransferDuration")
.since(V2_1)
.summary(
"Transfers up to this duration with the default walk speed value will be pre-calculated and included in the Graph."
"Transfers up to this duration with a mode-specific speed value will be pre-calculated and included in the Graph."
)
.asDuration(Duration.ofMinutes(30));
transferParametersForMode = TransferConfig.map(root, "transferParametersForMode");
maxStopToShapeSnapDistance =
root
.of("maxStopToShapeSnapDistance")
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
package org.opentripplanner.standalone.config.buildconfig;

import static org.opentripplanner.standalone.config.framework.json.OtpVersion.V2_7;

import java.util.EnumMap;
import java.util.Map;
import org.opentripplanner.graph_builder.module.TransferParameters;
import org.opentripplanner.routing.api.request.StreetMode;
import org.opentripplanner.standalone.config.framework.json.NodeAdapter;

public class TransferConfig {

public static Map<StreetMode, TransferParameters> map(NodeAdapter root, String parameterName) {
return root
.of(parameterName)
.since(V2_7)
.summary("Configures mode-specific properties for transfer calculations.")
.description(
"""
This field enables configuring mode-specific parameters for transfer calculations.
To configure mode-specific parameters, the modes should also be used in the `transferRequests` field in the build config.
Copy link
Member

Choose a reason for hiding this comment

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

Should we enfore this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The parsing method now throws an IllegalArgumentException when a mode is used in the transferParameters but not in the transferRequests


**Example**

```JSON
// build-config.json
{
"transferParametersForMode": {
"CAR": {
"disableDefaultTransfers": true,
"carsAllowedStopMaxTransferDuration": "3h"
},
"BIKE": {
"maxTransferDuration": "30m",
"carsAllowedStopMaxTransferDuration": "3h"
}
}
}
```
"""
)
.asEnumMap(StreetMode.class, TransferParametersMapper::map, new EnumMap<>(StreetMode.class));
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
package org.opentripplanner.standalone.config.buildconfig;

import static org.opentripplanner.standalone.config.framework.json.OtpVersion.V2_7;

import org.opentripplanner.graph_builder.module.TransferParameters;
import org.opentripplanner.standalone.config.framework.json.NodeAdapter;

public class TransferParametersMapper {
Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I added changes to both files


public static TransferParameters map(NodeAdapter c) {
TransferParameters.Builder builder = new TransferParameters.Builder();
builder.withMaxTransferDuration(
c
.of("maxTransferDuration")
.summary("This overwrites the default `maxTransferDuration` for the given mode.")
.since(V2_7)
.asDuration(TransferParameters.DEFAULT_MAX_TRANSFER_DURATION)
);
builder.withCarsAllowedStopMaxTransferDuration(
c
.of("carsAllowedStopMaxTransferDuration")
.summary(
"This is used for specifying a `maxTransferDuration` value to use with transfers between stops which are visited by trips that allow cars."
)
.description(
"""
This parameter configures additional transfers to be calculated for the specified mode only between stops that have trips with cars.
The transfers are calculated for the mode in a range based on the given duration.
By default, these transfers are not calculated unless specified for a mode with this field.

Calculating transfers only between stops that have trips with cars can be useful with car ferries, for example.
Using transit with cars can only occur between certain stops.
These kinds of stops require support for loading cars into ferries, for example.
The default transfers are calculated based on a configurable range (configurable by using the `maxTransferDuration` field)
which limits transfers from stops to only be calculated to other stops that are in range.
When compared to walking, using a car can cover larger distances within the same duration specified in the `maxTransferDuration` field.
This can lead to large amounts of transfers calculated between stops that do not require car transfers between them.
This in turn can lead to a large increase in memory for the stored graph, depending on the data used in the graph.

For cars, using this parameter in conjunction with `disableDefaultTransfers` allows calculating transfers only between relevant stops.
For bikes, using this parameter can enable transfers between ferry stops that would normally not be in range.
In Finland this is useful for bike routes that use ferries near the Turku archipelago, for example.
"""
)
.since(V2_7)
.asDuration(TransferParameters.DEFAULT_CARS_ALLOWED_STOP_MAX_TRANSFER_DURATION)
);
builder.withDisableDefaultTransfers(
c
.of("disableDefaultTransfers")
.summary("This disables default transfer calculations.")
Copy link
Member

Choose a reason for hiding this comment

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

You could add a bit more descriptive description for what the default transfer calculations means and why you might want to do this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I added more documentation

.description(
"""
The default transfers are calculated based on a configurable range (configurable by using the `maxTransferDuration` field)
which limits transfers from stops to only be calculated to other stops that are in range.
This parameter disables these transfers.
A motivation to disable default transfers could be related to using the `carsAllowedStopMaxTransferDuration` field which only
calculates transfers between stops that have trips with cars.
For example, when using the `carsAllowedStopMaxTransferDuration` field with cars, the default transfers can be redundant.
"""
)
.since(V2_7)
.asBoolean(TransferParameters.DEFAULT_DISABLE_DEFAULT_TRANSFERS)
);
return builder.build();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ public class NodeAdapterHelper {
new AnchorAbbreviation("od.", "osmDefaults."),
new AnchorAbbreviation("lfp.", "localFileNamePatterns."),
new AnchorAbbreviation("u.", "updaters."),
new AnchorAbbreviation("tpfm.", "transferParametersForMode."),
new AnchorAbbreviation("0.", "[0]."),
new AnchorAbbreviation("1.", "[1].")
);
Expand Down
Loading
Loading