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

Work towards removing TurnWeighting #1842

Merged
merged 5 commits into from
Jan 10, 2020
Merged

Conversation

easbar
Copy link
Member

@easbar easbar commented Dec 30, 2019

This is some preliminary work for #1820, mainly to reduce the diff when we actually fix #1820.

  1. (trivial) move the INFINITE_U_TURN_COSTS constant into Weighting.
  2. add Weighting#calcEdgeWeight/Millis. For now this is a pure convenience method that just calls calcWeight with prevOrNext=-1. For Remove TurnWeighting #1820 this will be the other way around: In AbstractWeighting we will have calcWeight which uses calcEdgeWeight and adds calcTurnWeight. Subclasses will then only override calcEdgeWeight/Millis and leave the turn cost calculation to AbstractWeighting. The turn cost calculation can then be adjusted by changing the TurnCostProvider implementation which is passed to AbstractWeighting via its constructor (at least this is what I currently think we should do).
  3. replace FlagEncoder#supports(TurnWeighting.class) with FlagEncoder#supportsTurnCosts(). Note that this it what it was like before 0.4 according to the changelog

The actual work for #1820 is happening here: https://github.com/graphhopper/graphhopper/tree/remove_turn_weighting, but its not in (near) a mergeable state yet.

Any thoughts ?

@easbar easbar added this to the 1.0 milestone Dec 30, 2019
# Conflicts:
#	core/src/main/java/com/graphhopper/routing/weighting/AbstractAdjustedWeighting.java
#	web-bundle/src/main/java/com/graphhopper/resources/InfoResource.java
@easbar
Copy link
Member Author

easbar commented Jan 10, 2020

Can we merge this ?

Copy link
Member

@karussell karussell left a comment

Choose a reason for hiding this comment

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

LGTM!

@@ -37,6 +39,8 @@
*/
double getMinWeight(double distance);

double calcEdgeWeight(EdgeIteratorState edgeState, boolean reverse);
Copy link
Member

Choose a reason for hiding this comment

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

Can you add javadocs? And highlight the difference to calcWeight?

Copy link
Member Author

Choose a reason for hiding this comment

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

Sure, good point!

@easbar easbar merged commit 63f00e1 into master Jan 10, 2020
@easbar easbar deleted the remove_turn_weighting_preliminary branch January 10, 2020 16:36
@karussell karussell mentioned this pull request Jan 10, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Remove TurnWeighting
2 participants