mirror of
https://github.com/mauriceboe/TREK.git
synced 2026-10-18 21:16:47 +00:00
fix(roadtrip): the alternatives list compared two routers' drive times
The ways of driving one leg come from OSRM, except the toll-free or motorway-free offer beside them: the shipped OSRM hosts are built without excludable road classes and answer exclude= with HTTP 400, so that one is asked of the avoidance router instead. Their figures were then read against each other. The quickest was elected across both, which is what decides the road drawn as the one being driven, and every other road was labelled with how far behind that election it sat. The two engines agree on the road to a tenth of a percent and not at all on how fast it may be driven: over twelve European legs the per-leg time ran from 14.7 % under OSRM's to 13.2 % over, with the sign following the region. On a Spanish motorway leg that gap alone is enough to crown a toll-free detour and call the motorway somebody is on the slow way round. An offer now carries the engine that timed it. It takes no part in electing the quickest, no difference is printed across the two, and the bar says where the figure came from. Each route still reports its own time; only the subtraction is refused. An instance running its own OSRM through the MLD pipeline answers exclude itself, so there both figures are one engine's and the comparison works exactly as before.
This commit is contained in:
@@ -649,6 +649,19 @@ export interface RouteAlternative {
|
||||
divergence: { lat: number; lng: number } | null
|
||||
/** Set when this way exists because a road class was left out of it. */
|
||||
avoids?: AvoidClass
|
||||
/**
|
||||
* Which engine priced this route, when it was not the one that priced the rest.
|
||||
*
|
||||
* Absent for everything OSRM answered, which is every route in a list except the
|
||||
* avoidance offer on a default install. It exists because the two engines do not
|
||||
* agree on speed: measured over twelve European legs the per-leg drive times run
|
||||
* from 14.7 % under OSRM's to 13.2 % over, and the sign depends on the region:
|
||||
* Valhalla is faster on Spanish autovía and slower through a city. So a figure from
|
||||
* one of them subtracted from a figure from the other is not a difference in
|
||||
* driving time, it is the gap between two speed models, and the reader has no way
|
||||
* of telling the two apart.
|
||||
*/
|
||||
engine?: 'valhalla'
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -684,10 +697,12 @@ const SAME_ROAD_KM = 0.5
|
||||
* Two lines are the same road.
|
||||
*
|
||||
* Length and duration used to decide this, and stopped being enough when a second engine
|
||||
* started answering. Valhalla prices roads differently — measured over ten European legs
|
||||
* its distances ran 2.8 % above OSRM's and its times 5.4 % — so on a 470 km leg the same
|
||||
* road comes back thirteen kilometres apart and a 200 m tolerance calls it a discovery.
|
||||
* Two identical blue lines on the map is exactly what that looks like.
|
||||
* started answering. Valhalla prices roads differently: measured over twelve European
|
||||
* legs its distances ran 1.8 % above OSRM's, so on a 470 km leg the same road comes back
|
||||
* thirteen kilometres apart and a 200 m tolerance calls it a discovery. Two identical
|
||||
* blue lines on the map is exactly what that looks like. Duration is the worse of the
|
||||
* two to lean on: over the same twelve legs it ranged from 14.7 % under OSRM's to 13.2 %
|
||||
* over, with the sign set by the region rather than by the road.
|
||||
*
|
||||
* The geometry does not care which engine drew it, so it decides whenever there is one.
|
||||
* The numbers stay as the fallback for a line with no shape to compare.
|
||||
@@ -778,6 +793,7 @@ async function valhallaExcluding(
|
||||
distance: leg.distance,
|
||||
duration: leg.duration,
|
||||
divergence: null,
|
||||
engine: 'valhalla',
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -22,10 +22,14 @@ import type { Waypoint, SnappedWaypoint, RouteAvoidClass } from '../../types'
|
||||
* - it refuses a point it cannot bind to a road, where OSRM still snaps one. The
|
||||
* Stephansdom, in a pedestrian zone, answers `No path could be found for input`
|
||||
* until a search radius is allowed;
|
||||
* - it prices roads differently. Over ten European legs its drive times sat 5.4 %
|
||||
* above OSRM's on average and 36 % above on one, and those times feed the arrival
|
||||
* clocks of a road trip day. Mixing engines inside one chain would move times
|
||||
* nobody asked to move.
|
||||
* - it prices roads differently, and not by a constant anybody could correct for.
|
||||
* Over twelve European legs the per-leg drive time ran from 14.7 % under OSRM's to
|
||||
* 13.2 % over, and the sign follows the region rather than the road: on Spanish
|
||||
* autovía Valhalla is the faster of the two on every leg measured, through a city
|
||||
* it is half again slower. Those times feed the arrival clocks of a road trip day,
|
||||
* so mixing engines inside one chain would move times nobody asked to move, and
|
||||
* subtracting one engine's figure from the other's, which is what an offer beside
|
||||
* a route invites, measures the gap between two speed models and nothing else.
|
||||
*
|
||||
* Which leaves exactly the shape this module serves: two points, one avoidance, asked
|
||||
* on demand, drawn as an offer next to the route rather than replacing it.
|
||||
|
||||
@@ -25,6 +25,7 @@ const overlay = (over: Partial<AlternativeOverlay> = {}): AlternativeOverlay =>
|
||||
duration: 10_800,
|
||||
distance: 290_000,
|
||||
slowerThanQuickest: 0,
|
||||
otherEngine: false,
|
||||
labelBg: '#0a84ff',
|
||||
at: { lat: 53, lng: 11 },
|
||||
...over,
|
||||
@@ -142,4 +143,19 @@ describe('RoadtripAlternativesBar', () => {
|
||||
fireEvent.click(screen.getByRole('button', { name: /close/i }))
|
||||
expect(onClose).toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('FE-ALTBAR-010: an offer the other engine timed says so, and the rest do not', () => {
|
||||
render(
|
||||
<RoadtripAlternativesBar
|
||||
open={leg()}
|
||||
overlays={[
|
||||
overlay({ note: 'Fastest' }),
|
||||
overlay({ index: 1, note: 'No tolls', otherEngine: true }),
|
||||
]}
|
||||
onChoose={vi.fn()}
|
||||
onClose={vi.fn()}
|
||||
/>,
|
||||
)
|
||||
expect(screen.getAllByLabelText(/avoidance router/i)).toHaveLength(1)
|
||||
})
|
||||
})
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
import React from 'react'
|
||||
import { Shuffle, X, AlertTriangle } from 'lucide-react'
|
||||
import { Shuffle, X, AlertTriangle, Info } from 'lucide-react'
|
||||
import { useTranslation } from '../../i18n/TranslationContext'
|
||||
import { Tooltip } from '../shared/Tooltip'
|
||||
import { useSettingsStore } from '../../store/settingsStore'
|
||||
import { formatDistance } from '../../utils/units'
|
||||
import { formatDurationShort } from './roadtripModel'
|
||||
@@ -92,6 +93,17 @@ export default function RoadtripAlternativesBar({
|
||||
: t('roadtrip.alt.slower', { time: formatDurationShort(alt.slowerThanQuickest) })}
|
||||
</span>
|
||||
</span>
|
||||
{/* Only where the drive time on the map came from the other engine. The
|
||||
road and the length are the same question either way; the time is not,
|
||||
and two engines' times sitting next to each other on the map invite a
|
||||
subtraction that means nothing. Saying so here, where the choice is
|
||||
actually made, is cheaper than a second request per offer to restate
|
||||
the whole list in one engine's terms. */}
|
||||
{alt.otherEngine ? (
|
||||
<Tooltip label={t('roadtrip.alt.otherEngine')}>
|
||||
<Info size={12} className="shrink-0 text-content-faint" aria-label={t('roadtrip.alt.otherEngine')} />
|
||||
</Tooltip>
|
||||
) : null}
|
||||
</button>
|
||||
))}
|
||||
</div>
|
||||
|
||||
@@ -143,4 +143,81 @@ describe('buildAlternativeOverlays', () => {
|
||||
)
|
||||
expect(out.filter(o => o.color === ALT_PRIMARY)).toHaveLength(1)
|
||||
})
|
||||
|
||||
it('FE-ALTOVL-011: a route the other engine priced cannot win the quickest election', () => {
|
||||
// The shape a default install actually produces: OSRM answered with the one road
|
||||
// it likes, and the toll-free offer beside it came from the avoidance router,
|
||||
// whose speed model reads a Spanish motorway a seventh faster. Electing across
|
||||
// the two paints the detour blue and calls the road being driven the slow way
|
||||
// round, on nothing but that gap.
|
||||
const out = buildAlternativeOverlays(
|
||||
[
|
||||
alt({ duration: 11_880, coordinates: line(20, 0) }),
|
||||
alt({ duration: 10_140, coordinates: line(20, 1), avoids: 'toll', engine: 'valhalla' }),
|
||||
],
|
||||
LABELS,
|
||||
)
|
||||
expect(out[0].color).toBe(ALT_PRIMARY)
|
||||
expect(out[1].color).toBe(ALT_SECONDARY)
|
||||
expect(out[0].note).toBe('Fastest')
|
||||
expect(out[1].note).toBe('No tolls')
|
||||
})
|
||||
|
||||
it('FE-ALTOVL-012: no difference is printed across two engines', () => {
|
||||
const out = buildAlternativeOverlays(
|
||||
[
|
||||
alt({ duration: 11_880, coordinates: line(20, 0) }),
|
||||
alt({ duration: 10_140, coordinates: line(20, 1), avoids: 'toll', engine: 'valhalla' }),
|
||||
],
|
||||
LABELS,
|
||||
)
|
||||
expect(out[0].slowerThanQuickest).toBe(0)
|
||||
expect(out[1].slowerThanQuickest).toBe(0)
|
||||
expect(out[0].otherEngine).toBe(false)
|
||||
expect(out[1].otherEngine).toBe(true)
|
||||
// Each route still reports its own time; it is only the subtraction that is refused.
|
||||
expect(out[0].duration).toBe(11_880)
|
||||
expect(out[1].duration).toBe(10_140)
|
||||
})
|
||||
|
||||
it('FE-ALTOVL-013: an instance whose own router answers still gets a real difference', () => {
|
||||
// A self-hosted OSRM built through the MLD pipeline answers `exclude` itself, so
|
||||
// the avoidance offer carries no engine of its own and the two figures are the
|
||||
// same engine's. That comparison is sound and must keep working.
|
||||
const out = buildAlternativeOverlays(
|
||||
[
|
||||
alt({ duration: 3600, coordinates: line(20, 0) }),
|
||||
alt({ duration: 5400, coordinates: line(20, 1), avoids: 'toll' }),
|
||||
],
|
||||
LABELS,
|
||||
)
|
||||
expect(out[1].slowerThanQuickest).toBe(1800)
|
||||
expect(out.every(o => o.otherEngine === false)).toBe(true)
|
||||
})
|
||||
|
||||
it('FE-ALTOVL-014: a list with nothing comparable in it still names one road primary', () => {
|
||||
const out = buildAlternativeOverlays(
|
||||
[
|
||||
alt({ duration: 3600, coordinates: line(20, 0), engine: 'valhalla' }),
|
||||
alt({ duration: 5400, coordinates: line(20, 1), avoids: 'toll', engine: 'valhalla' }),
|
||||
],
|
||||
LABELS,
|
||||
)
|
||||
expect(out.filter(o => o.color === ALT_PRIMARY)).toHaveLength(1)
|
||||
expect(out[0].color).toBe(ALT_PRIMARY)
|
||||
expect(out.every(o => o.slowerThanQuickest === 0)).toBe(true)
|
||||
})
|
||||
|
||||
it('FE-ALTOVL-015: the road being driven stays blue whatever the other engine says', () => {
|
||||
const out = buildAlternativeOverlays(
|
||||
[
|
||||
alt({ duration: 11_880, coordinates: line(20, 0), current: true }),
|
||||
alt({ duration: 10_140, coordinates: line(20, 1), avoids: 'toll', engine: 'valhalla' }),
|
||||
],
|
||||
LABELS,
|
||||
)
|
||||
expect(out[0].color).toBe(ALT_PRIMARY)
|
||||
expect(out[0].note).toBe('Current')
|
||||
expect(out[1].color).toBe(ALT_SECONDARY)
|
||||
})
|
||||
})
|
||||
|
||||
@@ -16,6 +16,13 @@ export interface AlternativeOverlay {
|
||||
distance: number
|
||||
/** Seconds more than the quickest offer; negative never happens by construction. */
|
||||
slowerThanQuickest: number
|
||||
/**
|
||||
* True when this route's figures came from the other engine, so they cannot be read
|
||||
* against the rest of the list. Nothing derived from a comparison is filled in for
|
||||
* such a route: it takes no part in electing the quickest and its
|
||||
* `slowerThanQuickest` stays zero.
|
||||
*/
|
||||
otherEngine: boolean
|
||||
labelBg: string
|
||||
/** Where to hang the label — a point on this route and on no other. */
|
||||
at: { lat: number; lng: number }
|
||||
@@ -91,7 +98,21 @@ export function buildAlternativeOverlays(
|
||||
|
||||
// The quickest of what came back, which is what Apple calls out — not necessarily the
|
||||
// first entry, since the road currently driven is put at the top when there is one.
|
||||
const quickest = routes.reduce((best, r, i) => (r.duration < routes[best].duration ? i : best), 0)
|
||||
//
|
||||
// Only among the routes one engine priced. The avoidance offer on a default install
|
||||
// comes from the second engine, whose speed model differs by up to a seventh either
|
||||
// way depending on the region, so letting it into this election decides which road is
|
||||
// blue and how much slower every other road is called on nothing but that gap. On a
|
||||
// Spanish motorway leg that is enough to crown a toll-free B-road detour and label
|
||||
// the motorway somebody is actually driving as the slower way round.
|
||||
//
|
||||
// Index 0 is the fallback rather than -1: the first entry is the road being driven
|
||||
// when there is one and the router's own pick otherwise, so if a list ever held
|
||||
// nothing but second-engine routes, that is still the one to call primary.
|
||||
const comparable = routes.map((r, i) => (r.engine ? -1 : i)).filter(i => i >= 0)
|
||||
const quickest = comparable.length
|
||||
? comparable.reduce((best, i) => (routes[i].duration < routes[best].duration ? i : best), comparable[0])
|
||||
: 0
|
||||
const anyCurrent = routes.some(r => r.current)
|
||||
|
||||
return routes.map((route, index) => {
|
||||
@@ -100,6 +121,7 @@ export function buildAlternativeOverlays(
|
||||
// Blue is the road you are on: the one currently driven, or the router's own pick
|
||||
// when nothing has been bent. Everything else is the pale blue of an offer.
|
||||
const primary = route.current || (!anyCurrent && index === quickest)
|
||||
const otherEngine = !!route.engine
|
||||
return {
|
||||
index,
|
||||
coordinates: route.coordinates,
|
||||
@@ -119,7 +141,14 @@ export function buildAlternativeOverlays(
|
||||
: index === quickest ? labels.fastest : '',
|
||||
duration: route.duration,
|
||||
distance: route.distance,
|
||||
slowerThanQuickest: Math.max(0, Math.round(route.duration - routes[quickest].duration)),
|
||||
// Left at zero for a route the other engine priced, and for every route when
|
||||
// that is the only thing to measure against. The bar reads a zero as "no
|
||||
// difference worth printing" and falls back to naming what the road is, which
|
||||
// for these is always the class left out of it.
|
||||
slowerThanQuickest: otherEngine || !!routes[quickest].engine
|
||||
? 0
|
||||
: Math.max(0, Math.round(route.duration - routes[quickest].duration)),
|
||||
otherEngine,
|
||||
labelBg: primary ? ALT_LABEL_PRIMARY_BG : ALT_LABEL_SECONDARY_BG,
|
||||
at: at ?? { lat: 0, lng: 0 },
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user