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

Path comparison updates #2343

Merged
merged 6 commits into from
May 1, 2020
Merged

Path comparison updates #2343

merged 6 commits into from
May 1, 2020

Conversation

gknisely
Copy link
Member

@gknisely gknisely commented May 1, 2020

Issue

At some point these two binaries got out of date. Updated.

Sample command
./valhalla_path_comparison -s 'crjgkA~lthpCLiMxBofBgm@{AgM?uZxCU?' -t auto --config ../../conf/valhalla.conf

+++++++++++++++++++++++++++++++++++++
wayid: 11929406
name: West Main Street
+++++++++++++++++++++++++++++++++++++

----------Edge----------
Edge GraphId: 0/2905/191349
Edge length: 130
EdgeCost cost: 10.8225 secs: 11.7
------------------------

-------Transition-------
Pred GraphId: 0/2905/191349
TransitionCost cost: 0 secs: 0
------------------------

----------Edge----------
Edge GraphId: 0/2905/192090
Edge length: 141
EdgeCost cost: 11.7383 secs: 12.69
------------------------

+++++++++++++++++++++++++++++++++++++
wayid: 76951972
name: North Decatur Street
+++++++++++++++++++++++++++++++++++++

-------Transition-------
Pred GraphId: 0/2905/192090
TransitionCost cost: 7.5 secs: 7.5
------------------------

----------Edge----------
Edge GraphId: 0/2905/191361
Edge length: 202
EdgeCost cost: 16.8165 secs: 18.18
------------------------

+------------------------------------------------------------------------+
| Total Edge Cost       :    39.3773  Total Edge Secs       :      42.57 |
| Total Transition Cost :        7.5  Total Transition Secs :        7.5 |
| Total Cost            :    46.8773  Total Secs            :      50.07 |
+------------------------------------------------------------------------+

Tasklist

  • Generally use squash merge to rebase and clean comments before merging
  • Update the changelog

Requirements / Relations

Link any requirements here. Other pull requests this PR is based on?

@danpat
Copy link
Member

danpat commented May 1, 2020

@gknisely can you add a test for these so we don't get future similar regressions?

Copy link
Member

@kdiluca kdiluca left a comment

Choose a reason for hiding this comment

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

I tested this with my examples and is working! 👍

@gknisely
Copy link
Member Author

gknisely commented May 1, 2020

@gknisely can you add a test for these so we don't get future similar regressions?

@danpat

@kevinkreiser is going to add an issue to rewrite these programs to use the actor interface. We can do it then.

@gknisely gknisely merged commit d042cf5 into master May 1, 2020
@gknisely gknisely deleted the path_comparison_updates branch February 22, 2024 13:31
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

4 participants