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

Improves value function performance #42

Merged
merged 6 commits into from
May 15, 2024

Conversation

merschformann
Copy link
Member

@merschformann merschformann commented May 14, 2024

Description

Fixes a performance regression that surfaces under particular circumstances that was caused by extra checks in the value function and the new (more generic) stop generation logic.

Changes

  • Added a check for travel distance validity before accessing value function in factory/format.go.
  • Simplified value functions.
  • Avoids some interfaces in stop generation.
  • Uses old (faster) logic when no forced neighbors or "disallowed" successors exist.
  • Improved new logic by accessing info more directly and avoiding some funcs if possible.

@merschformann merschformann changed the title Enhanced Distance Calculation Logic in Routing Engine Improved value function performance May 14, 2024
@merschformann merschformann changed the title Improved value function performance Improves value function performance May 14, 2024
Comment on lines +149 to +151
hasTravelDistance := solutionStop.Previous().ModelStop().Location().IsValid() &&
solutionStop.ModelStop().Location().IsValid()
if data, ok := solutionStop.Vehicle().ModelVehicle().VehicleType().Data().(vehicleTypeData); ok && hasTravelDistance {
Copy link
Member Author

Choose a reason for hiding this comment

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

This is the fix for the removed nil and IsValid checks (value funcs).

Copy link
Contributor

Choose a reason for hiding this comment

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

@merschformann this PR will make the code also faster likely

Copy link
Member Author

Choose a reason for hiding this comment

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

Should we wait on that one? Is it close?

Copy link
Contributor

Choose a reason for hiding this comment

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

It is done but has not been benchmarked on larger instances yet

Copy link
Member Author

Choose a reason for hiding this comment

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

I can quickly do that in our current experiments. I will also run a larger one anyway and will include it there as well. 👍
Give me some time.

@merschformann merschformann merged commit f56ea11 into develop May 15, 2024
7 checks passed
@merschformann merschformann deleted the merschformann/fix-value-performance-regression branch May 15, 2024 20:09
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.

3 participants