Skip to content

Fix high walk reluctance leading to zero egress results for rental searches#5605

Merged
leonardehrenfried merged 6 commits into
dev-2.xfrom
max-count-rental
Jan 11, 2024
Merged

Fix high walk reluctance leading to zero egress results for rental searches#5605
leonardehrenfried merged 6 commits into
dev-2.xfrom
max-count-rental

Conversation

@leonardehrenfried

Copy link
Copy Markdown
Member

Summary

@binh-dam-ibigroup has reported that there are some cases where rental searches with a very high walk reluctance lead to zero egress legs.

After some investigation I found out that it's unfortunate chain circumstances that conspire to produce this problem.

  • during the egress/arriveBy search the speculative rental states crowd out the regular walk states as they have such a high reluctance
  • the speculative rental states reach the required number of stops but none of them are actually allowed to board the vehicle as no vehicle has been found
  • the termination requirements are met but none of the states are usable for egress results

Luckily, there is a simple fix for the problem: when counting stops, also check State#isFinal to make sure that only allowable states count towards the number of stops.

This PR also contains small cleanups of the A* code.

Unit tests

✔️

Documentation

Lots of Javadoc added and updated.

@leonardehrenfried leonardehrenfried requested a review from a team as a code owner January 5, 2024 21:50
@leonardehrenfried leonardehrenfried added !Bug Apply to issues describing a bug and PRs witch fixes it. IBI Developed by or important for IBI Group labels Jan 5, 2024
@leonardehrenfried leonardehrenfried changed the title Max count rental Fix high walk reluctance leading to zero egress results for rental results Jan 5, 2024
@codecov

codecov Bot commented Jan 5, 2024

Copy link
Copy Markdown

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (d71c768) 67.45% compared to head (8a67be4) 67.46%.
Report is 58 commits behind head on dev-2.x.

Additional details and impacted files
@@              Coverage Diff              @@
##             dev-2.x    #5605      +/-   ##
=============================================
+ Coverage      67.45%   67.46%   +0.01%     
- Complexity     16188    16196       +8     
=============================================
  Files           1862     1862              
  Lines          71184    71213      +29     
  Branches        7401     7406       +5     
=============================================
+ Hits           48019    48046      +27     
+ Misses         20674    20673       -1     
- Partials        2491     2494       +3     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@leonardehrenfried leonardehrenfried changed the title Fix high walk reluctance leading to zero egress results for rental results Fix high walk reluctance leading to zero egress results for rental searches Jan 5, 2024

@binh-dam-ibigroup binh-dam-ibigroup left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I tested doing the original search that led to the issue and it seems to be fixed! Code changes are very clean.

}

/**
* Creates a state starts the scooter rental but in arriveBy mode, so therefore starting with

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The first part of this comment does not make sense.

vesameskanen
vesameskanen previously approved these changes Jan 11, 2024
@vesameskanen

Copy link
Copy Markdown
Contributor

Nice that the fix is so simple! I verified that isFinal() call does not cause any significant routing time penalties.

@leonardehrenfried

Copy link
Copy Markdown
Member Author

I also ran the speed test on the branch and could not see a slowdown.

@leonardehrenfried

Copy link
Copy Markdown
Member Author

Since the only review comment was about Javadoc not making sense, I fixed it and have taken the liberty to merge without another round of review.

@leonardehrenfried leonardehrenfried merged commit 75462f0 into dev-2.x Jan 11, 2024
@leonardehrenfried leonardehrenfried deleted the max-count-rental branch January 11, 2024 09:10
t2gran pushed a commit that referenced this pull request Jan 11, 2024
@t2gran t2gran added this to the 2.5 (next release) milestone Mar 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

!Bug Apply to issues describing a bug and PRs witch fixes it. IBI Developed by or important for IBI Group

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants