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

allow walk access or egress for ride_hail_transit #3818

Open
wants to merge 17 commits into
base: develop
Choose a base branch
from

Conversation

zneedell
Copy link
Collaborator

@zneedell zneedell commented Jan 18, 2024

Previously we only allowed itineraries of rh -> transit -> rh when the mode ride_hail_transit was selected. This should allow for either the access or egress leg to be walk. There's nothing explicitly preventing the router from constructing a walk_transit itinerary as well, but that one should be filtered out in ChoosesMode if it is not allowed based on the agent's input plans.

This PR also captures RH cost in router so that we don't have as many routes with really long ride-hail legs and short transit legs


This change is Reviewable

@zneedell
Copy link
Collaborator Author

Test!

@zneedell
Copy link
Collaborator Author

Test!

@zneedell
Copy link
Collaborator Author

Test!

@zneedell
Copy link
Collaborator Author

Test!

Copy link
Collaborator

@jlaz jlaz left a comment

Choose a reason for hiding this comment

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

See comment on r5Wrapper.travelCostCalculator

src/main/scala/beam/router/r5/R5Wrapper.scala Show resolved Hide resolved
@zneedell
Copy link
Collaborator Author

Test!

Copy link
Collaborator

@dimaopen dimaopen left a comment

Choose a reason for hiding this comment

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

Maybe it's time to refactor ChoosesModeData. Instead of multiple requestIds and Optional responses that also may contain a virtual "Error" not requested, we could have a map similar to parkingResponses map. That map contains all the responses from the router and the RHM. And when all the required keys are presented in the map then we can do our caclulation and finish the mode choice?

): Vector[EmbodiedBeamTrip] = {
if (
rideHail2TransitAccessResult.error.isEmpty &&
): Option[EmbodiedBeamTrip] = {
Copy link
Collaborator

Choose a reason for hiding this comment

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

This method was supposed to return multiple RH2T itineraries including pooling/solo for access/egress combinations.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Aah, that makes sense, thanks. Hopefully the updates should capture that behavior now

// really need to send separate RH requests for each RH_TRANSIT itinerary if we wanted to do it correctly. This
// sounds too complicated and costly, so instead we just run a route choice on the RH_TRANSIT itineraries returned
// by R5. These don't necessarily have correct wait times, but R5 has been updated to give them appropriate costs.
// Once we've chosen the best itinerary we can send requests to the RHM to fill in true costs and wait times
Copy link
Collaborator

Choose a reason for hiding this comment

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

If we generally have the same response from the router (except it may contain WALK as access or egress) then the rest of the ChoosesMode should be the same (except we don't need to send RH requests for WALK legs).

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I'm imagining a situation where the router returns two separate WALK->TRANSIT->RH itineraries, where the two egress RH legs pick up in different locations (e.g. at different transit stops). As I, understand it doing this would allow for only one RH request for the egress leg in this case, even though it could start from multiple locations. I'm not actually sure what the consequences of that would be (only having a maybe incorrect estimated wait time for the egress leg?), but it seems to me like we'd still be missing something.

choosesModeData.rideHail2TransitEgressResult.nonEmpty
choosesModeData.copy(
rideHailResult = Some(theRideHailResult),
routingFinished = choosesModeData.routingFinished || routingFinished
Copy link
Collaborator

Choose a reason for hiding this comment

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

I remeber introducing this routingFinished but I don't see it is used now. Am I missing something?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It seems like it's still used here, although I haven't fully understood the logic

routingFinished = routingRequestMap.isEmpty,

@zneedell
Copy link
Collaborator Author

Test now!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants