Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 2 additions & 0 deletions MEMORY.md
Original file line number Diff line number Diff line change
Expand Up @@ -37,3 +37,5 @@ Personal working preferences (not project facts) live in Claude Code's per-user
[LEARN:claude] A user-level skill (`~/.claude/skills/<name>`) takes precedence over a project skill with the same name, so r5r-specific skill copies never run for anyone who has the user-level twin. Put r5r specifics in `AGENTS.md`, the path-scoped rules, and `.claude/agents/` (project agents DO override user-level agents).

[LEARN:claude] Rule frontmatter must use `paths:`; Cursor-style `globs:` / `alwaysApply:` are ignored, so such a rule loads in every session.

[LEARN:r5r] `start_r5r_java()` called `rJava::.jinit(parameters = c(log_path, ...))`, which REPLACES `.jinit()`'s default `getOption("java.parameters")`, so a user's `-Xmx` was silently ignored from v2.2.0 to v2.4.0 (JVM got its default heap; `.onAttach`/`r5r_sitrep()` still echoed the option). Fixed in dev by prepending `getOption("java.parameters")`. To verify JVM flags, read `RuntimeMXBean$getInputArguments()` / `Runtime$maxMemory()` in a fresh R session; never trust the option value. To benchmark CRAN <= 2.4.0 with a set heap, call `.jinit()` yourself with the flags plus `-DLOG_PATH/-DR5_VER/-DR5R_VER` before `build_network()`.
73 changes: 70 additions & 3 deletions java-r5rcore/src/org/ipea/r5r/Planner/TripPlanner.java
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,13 @@
import com.conveyal.r5.transit.TransportNetwork;
import gnu.trove.iterator.TIntObjectIterator;
import gnu.trove.map.TIntIntMap;
import gnu.trove.map.TObjectIntMap;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import java.lang.reflect.Field;
import java.util.*;
import java.util.function.BiConsumer;
import java.util.function.IntFunction;
import java.util.stream.Collectors;

Expand Down Expand Up @@ -64,8 +67,8 @@ public List<Trip> plan() {
accessRouter = findAccessPaths(request);
egressRouter = findEgressPaths(request);

Map<LegMode, TIntIntMap> accessTimes = accessRouter.entrySet().stream()
.collect(Collectors.toMap(Map.Entry::getKey, e -> e.getValue().getReachedStops()));
AccessTimes accessTimes = new AccessTimes(); // resets R5's target pruning per draw, see AccessTimes
accessRouter.forEach((mode, r) -> accessTimes.put(mode, r.getReachedStops()));
Map<LegMode, TIntIntMap> egressTimes = egressRouter.entrySet().stream()
.collect(Collectors.toMap(Map.Entry::getKey, e -> e.getValue().getReachedStops()));

Expand All @@ -81,14 +84,37 @@ public List<Trip> plan() {
// in fact, we have been moving in the opposite direction with leap-second smearing
departureTime + request.maxTripDurationMinutes * FastRaptorWorker.SECONDS_PER_MINUTE);
} else {
listSupplier = (t) -> new SuboptimalDominatingList(Math.max(request.suboptimalMinutes, 0));
// R5 keeps states up to the end of the time window + max_trip_duration for every draw, but plan() drops
// trips longer than max_trip_duration from the draw's departure t: reject those states early, as the
// FareDominatingList above does with its maxClockTime
// with shortest_path only the fastest trip is returned, so the limit shrinks to the duration of the best
// trip found so far: direct trips, then each draw's arrivals at the destination (stop == -1)
final int[] maxDuration = {request.maxTripDurationMinutes * FastRaptorWorker.SECONDS_PER_MINUTE};
if (shortestPath) trips.values().forEach(trip -> maxDuration[0] = Math.min(maxDuration[0], trip.getTotalDurationSeconds()));
listSupplier = (t) -> new SuboptimalDominatingList(Math.max(request.suboptimalMinutes, 0)) {
@Override
public boolean add(McRaptorSuboptimalPathProfileRouter.McRaptorState state) {
if (state.time - t > maxDuration[0] || !super.add(state)) return false;
if (shortestPath && state.stop == -1) maxDuration[0] = Math.min(maxDuration[0], state.time - t);
return true;
}
};
}

McRaptorSuboptimalPathProfileRouter router = new McRaptorSuboptimalPathProfileRouter(transportNetwork,
request, accessTimes, egressTimes, listSupplier,
null, true);

int maxFare = request.maxFare;
if (request.inRoutingFareCalculator == null) {
request.maxFare = -1; // turns on R5's target pruning, see AccessTimes
accessTimes.bestTimesAtTarget = bestTimesAtTarget(router);
}
router.route();
request.maxFare = maxFare; // the fare filter below needs the original value
if (accessTimes.bestTimesAtTarget != null && accessTimes.draws != request.monteCarloDraws) {
throw new IllegalStateException("R5's McRAPTOR router no longer works as TripPlanner.AccessTimes expects");
}

for (TIntObjectIterator<Collection<McRaptorSuboptimalPathProfileRouter.McRaptorState>> it =
router.finalStatesByDepartureTime.iterator(); it.hasNext();) {
Expand Down Expand Up @@ -239,4 +265,45 @@ private Map<LegMode, StreetRouter> findEgressPaths(ProfileRequest request) {
return egressRouter;
}

/**
* Access times given to R5's McRAPTOR router. They also make R5's target pruning safe for r5r.
* <p>
* R5 drops a state that arrives later than the best arrival at the destination found so far (plus
* suboptimal_minutes), but only when request.maxFare &lt; 0 (McRaptorSuboptimalPathProfileRouter.addState, R5 v7.5).
* Without a fare structure r5r passes maxFare = Integer.MAX_VALUE, so this pruning was off and every Monte Carlo
* draw searched the whole network up to max_trip_duration. plan() therefore sets maxFare = -1 during route().
* <p>
* R5 keeps that best arrival (private field bestTimesAtTargetByAccessMode) across draws, so an early draw would
* prune the trips of later draws and change results. route() calls accessTimes.forEach() exactly once per draw,
* after bestStates.clear() and before any state of the draw is added, so clearing the field there limits the
* pruning to the current draw. Pruned states can only lead to arrivals that the destination bag drops anyway, so
* results are identical to the unpruned search.
* <p>
* Re-check on every R5 upgrade: the field name (a rename fails loudly in bestTimesAtTarget()); the maxFare &lt; 0
* condition in addState (if it changes, pruning is silently off: slower, same results); and the forEach() call
* once per draw (checked against monteCarloDraws after route()).
*/
private static final class AccessTimes extends HashMap<LegMode, TIntIntMap> {
TObjectIntMap<?> bestTimesAtTarget; // R5's bestTimesAtTargetByAccessMode, null when pruning is off
int draws;

@Override
public void forEach(BiConsumer<? super LegMode, ? super TIntIntMap> action) {
if (bestTimesAtTarget != null) {
bestTimesAtTarget.clear(); // its no-entry value is Integer.MAX_VALUE: "no arrival yet"
draws++;
}
super.forEach(action);
}
}

private static TObjectIntMap<?> bestTimesAtTarget(McRaptorSuboptimalPathProfileRouter router) {
try {
Field field = McRaptorSuboptimalPathProfileRouter.class.getDeclaredField("bestTimesAtTargetByAccessMode");
field.setAccessible(true);
return (TObjectIntMap<?>) field.get(router);
} catch (ReflectiveOperationException e) {
throw new IllegalStateException("R5's McRAPTOR router no longer works as TripPlanner.AccessTimes expects", e);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,9 @@ private void populateDataFrame(OneOriginResult travelTimeResults, RDataFrame tra
}

private void populateRegularResults(OneOriginResult travelTimeResults, RDataFrame travelTimesTable) {
// column names built once, not per destination (String.format was ~3% of CPU time with 5 percentiles)
String[] columns = new String[this.routingProperties.percentiles.length];
for (int p = 0; p < columns.length; p++) columns[p] = String.format("travel_time_p%02d", this.routingProperties.percentiles[p]);
for (int destination = 0; destination < travelTimeResults.travelTimes.nPoints; destination++) {
if (travelTimeResults.travelTimes.getValues()[0][destination] <= maxTripDuration) {

Expand All @@ -143,9 +146,8 @@ private void populateRegularResults(OneOriginResult travelTimeResults, RDataFram
// set percentiles
for (int p = 0; p < this.routingProperties.percentiles.length; p++) {
int tt = travelTimeResults.travelTimes.getValues()[p][destination];
String ps = String.format("%02d", this.routingProperties.percentiles[p]);
if (tt <= maxTripDuration) {
travelTimesTable.set("travel_time_p" + ps, tt);
travelTimesTable.set(columns[p], tt);
}
}
}
Expand Down
4 changes: 3 additions & 1 deletion java-r5rcore/src/org/ipea/r5r/R5/R5TravelTimeComputer.java
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,9 @@ public OneOriginResult computeTravelTimes() {
// Change to walking in order to reach transit stops in pedestrian-only areas like train stations.
// This implies you are dropped off or have a very easy parking spot for your vehicle.
// This kind of multi-stage search should also be used when building egress distance cost tables.
if (accessMode != StreetMode.WALK) {
// Only with transit, as in upstream R5 v7.5 (TravelTimeComputer): this walk can take as long as the car/bike
// search itself, and without transit it would let car-only trips end with a walk not bounded by max_walk_time.
if (accessMode != StreetMode.WALK && request.hasTransit()) {
sr.keepRoutingOnFoot();
}

Expand Down
3 changes: 3 additions & 0 deletions r-package/NEWS.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

**Bug fixes**

- Java memory set with `options(java.parameters = "-Xmx...")` before `library(r5r)` was ignored since r5r v2.2.0, so Java always ran with its default maximum heap (a quarter of the machine's RAM, capped at about 30 GB). r5r started Java with its own parameters only (log path and versions), which replaced the user's. They are now combined, so the memory limit and any other Java options set by the user take effect again.
- Fix bug related to optimization of origins and destinations. Closes again [#501](https://github.com/ipeaGIT/r5r/issues/501).
- [#569](https://github.com/ipeaGIT/r5r/pull/569) Fix broken source links in documentation website. Closed [#527](https://github.com/ipeaGIT/r5r/issues/527)
- Origins/destinations passed as `sf` objects that also carried `lon`/`lat` (or `x`/`y`) attribute columns were routed using those attribute columns instead of the point geometry. The geometry is now always used.
Expand Down Expand Up @@ -38,7 +39,9 @@

**Minor changes**

- Car-only and bicycle-only routing in `travel_time_matrix()`, `expanded_travel_time_matrix()`, `arrival_travel_time_matrix()`, `accessibility()` and `pareto_frontier()` is 2.5 to 4 times faster. After each car or bicycle search, r5r ran a second search on foot that R5 only needs to reach public transport stops. Bicycle results are unchanged. Car-only trips can no longer end with a walk that `max_walk_time` did not limit (e.g. against a one-way street), so some car travel times are now longer (3% of origin-destination pairs in the Porto Alegre sample data, by 1 to 8 minutes), now consistent with `isochrone()`.
- `detailed_itineraries()` now builds street paths and geometries only for the itineraries it returns, instead of for every candidate found by the router (including those later dropped by `max_trip_duration` or `shortest_path`). Results are identical. In benchmarks with the Porto Alegre sample data, the gain in computation time was within run-to-run noise, since most of the time is spent in the routing search itself.
- `detailed_itineraries()` with public transport is about 1.5 times faster with `max_trip_duration = 60`, 2.4 times faster with `time_window = 30`, and about 4 times faster with the defaults (`max_trip_duration = 120`, `shortest_path = TRUE`), and its peak memory use is lower. R5's router can stop exploring routes that already arrive later than the best arrival found at the destination, but r5r had this turned off whenever no `fare_structure` was used. It is now on, applied separately to each departure time within `time_window`, and routes longer than `max_trip_duration` from their own departure time (or, with `shortest_path = TRUE`, longer than the fastest itinerary already found) are dropped during the search instead of afterwards. The itineraries returned are unchanged; only with `suboptimal_minutes > 0` can the `option` numbers of two itineraries with exactly the same duration and number of segments be swapped. Routing with a `fare_structure` is unchanged.
- The deprecated `r5r_core` argument is now the last argument of every function. It used to be the second one, so positional calls such as `travel_time_matrix(r5r_network, origins, destinations)` bound `origins` to `r5r_core` and failed with a misleading deprecation warning and error.
- Documentation fixes: `carspeed_scale` defaults to `1` (not `NULL`), and the unsupported modes `BICYCLE_RENT` and `CAR_PARK` were removed from the list of transport modes.
- The documentation of `detailed_itineraries()` now lists the output columns and how itinerary options are ordered, and describes the `osm_link_ids` columns correctly (`board_stop_id` and `alight_stop_id` are feed-prefixed GTFS stop ids, not OSM ids; `edge_id_list` is also added). It also explains that `shortest_path = TRUE` picks the itinerary with the shortest travel time among all departures within `time_window` (not the earliest arrival), and that `suboptimal_minutes` requires `shortest_path = FALSE` and no `fare_structure`.
Expand Down
3 changes: 2 additions & 1 deletion r-package/R/utils.R
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,8 @@ start_r5r_java <- function(data_path,
r5_version <- paste0("-DR5_VER=", r5r_env$r5_jar_version)
r5r_version <- paste0("-DR5R_VER=", utils::packageVersion("r5r"))

rJava::.jinit(parameters = c(log_path, r5_version, r5r_version))
# passing `parameters` replaces .jinit()'s default, so keep the user's options(java.parameters)
rJava::.jinit(parameters = c(getOption("java.parameters"), log_path, r5_version, r5r_version))

ver <- get_java_version()

Expand Down
Binary file modified r-package/inst/jar/r5r.jar
Binary file not shown.
Loading