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

Fixes missing best path update for bidir algos when there is a loop at the meeting point (#1592) #1606

Merged
merged 10 commits into from May 6, 2019

Conversation

@easbar
Copy link
Collaborator

easbar commented May 3, 2019

  • Fixes #1592.
  • removes no longer needed TraversalMode.EDGE_BASED_1DIR (see #1520)
easbar added 6 commits Apr 2, 2019
@easbar

This comment has been minimized.

Copy link
Collaborator Author

easbar commented May 3, 2019

Hmm not sure what exactly failed the build for JDK11, but there are several warnings like this:

WARNING: HK2 service reification failed for [org.glassfish.jersey.message.internal.DataSourceProvider] with an exception:
MultiException stack 1 of 2
java.lang.NoClassDefFoundError: javax/activation/DataSource

?

@karussell

This comment has been minimized.

Copy link
Member

karussell commented May 3, 2019

@michaz could it be that this commit e8b5db9#diff-4db58d2546fb73b749d75a3df93f1c10 now requires this javax dependency in the reader-gtfs module too like in #1388? Or are some of the dependencies supposed to be test scope?

easbar added 2 commits May 3, 2019
@karussell karussell added this to the 0.13 milestone May 3, 2019
@easbar easbar changed the title Fixes missing best path update for bidir algos when there is a loop at the meeting point (#1592) WIP: Fixes missing best path update for bidir algos when there is a loop at the meeting point (#1592) May 3, 2019
@easbar easbar changed the title WIP: Fixes missing best path update for bidir algos when there is a loop at the meeting point (#1592) Fixes missing best path update for bidir algos when there is a loop at the meeting point (#1592) May 4, 2019
@michaz

This comment has been minimized.

Copy link
Member

michaz commented May 5, 2019

I liberally use dropwizard from reader-gtfs, yes.

@michaz

This comment has been minimized.

Copy link
Member

michaz commented May 5, 2019

Ah, I thought we had found out that Dropwizard doesn't support newer Javas yet, and would wait for their update until we really support them?

@michaz

This comment has been minimized.

Copy link
Member

michaz commented May 5, 2019

But if that activation block is the only thing that's missing, then sure, let's put it in. Totally forgot about that.

@easbar

This comment has been minimized.

Copy link
Collaborator Author

easbar commented May 6, 2019

Sorry, which activation block is missing ?

@michaz

This comment has been minimized.

Copy link
Member

michaz commented May 6, 2019

What Peter was refering to was adding

<dependency>
            <groupId>javax.activation</groupId>
            <artifactId>activation</artifactId>
            <version>1.1.1</version>
        </dependency>

to the pom of reader-gtfs.

Don't know if that helps, and also don't know why the lack of that would fail on your branch but not on master.

easbar added 2 commits May 6, 2019
@easbar

This comment has been minimized.

Copy link
Collaborator Author

easbar commented May 6, 2019

Don't know if that helps, and also don't know why the lack of that would fail on your branch but not on master.

Ok thanks! Looks like the warnings are gone with this, but no idea why there should be a difference to master.

Now the build passed. However I had to start it twice, the first time AppVeyor failed with this:

[ERROR] Tests run: 7, Failures: 0, Errors: 1, Skipped: 0, Time elapsed: 11.249 s <<< FAILURE! - in com.graphhopper.api.GraphHopperGeocodingIT
[ERROR] testExtent(com.graphhopper.api.GraphHopperGeocodingIT)  Time elapsed: 5.013 s  <<< ERROR!
java.lang.RuntimeException: Problem performing geocoding for https://graphhopper.com/api/1/geocode?reverse=false&q=new york&limit=7&locale=en&provider=default&key=78da6e9a-273e-43d1-bdda-8f24e007a1fa: connect timed out
	at com.graphhopper.api.GraphHopperGeocodingIT.testExtent(GraphHopperGeocodingIT.java:38)
Caused by: java.net.SocketTimeoutException: connect timed out
	at com.graphhopper.api.GraphHopperGeocodingIT.testExtent(GraphHopperGeocodingIT.java:38)

and also this:

[ERROR] testPathDetails(com.graphhopper.api.GraphHopperWebIT)  Time elapsed: 5.012 s  <<< ERROR!
java.lang.RuntimeException: Problem while fetching path [49.6724,11.3494, 49.655,11.418]: connect timed out
	at com.graphhopper.api.GraphHopperWebIT.testPathDetails(GraphHopperWebIT.java:310)
Caused by: java.net.SocketTimeoutException: connect timed out
	at com.graphhopper.api.GraphHopperWebIT.testPathDetails(GraphHopperWebIT.java:310)

Looks like it has a 5s time limit ?

@@ -465,7 +465,7 @@ public static EdgeIteratorState getEdge(Graph graph, int base, int adj) {
public static int createEdgeKey(int nodeA, int nodeB, int edgeId, boolean reverse) {
edgeId = edgeId << 1;
if (reverse)
return (nodeA > nodeB) ? edgeId : edgeId + 1;
return (nodeA >= nodeB) ? edgeId : edgeId + 1;

This comment has been minimized.

Copy link
@karussell

karussell May 6, 2019

Member

Ah, this is a nice fix! (is it related to this issue?)

This comment has been minimized.

Copy link
@easbar

easbar May 6, 2019

Author Collaborator

Yes this is one of the two fixes needed here. Imagine a loop edge with nodeA==nodeB. If the edge key is not independent of reverse the bwd search does not find the SPTEntry of the fwd search in updateBestPath and vice versa.

@karussell

This comment has been minimized.

Copy link
Member

karussell commented May 6, 2019

Don't know if that helps, and also don't know why the lack of that would fail on your branch but not on master.

Uhm, this is indeed strange as the travis config files are the same.

However I had to start it twice, the first time AppVeyor failed with this

Thanks for letting us know. Will investigate if there is a hidden problem in our API service.

@easbar easbar merged commit 57a4bc6 into master May 6, 2019
3 checks passed
3 checks passed
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
@easbar easbar deleted the bidir_turn_restrictions_bug branch May 7, 2019
Anvoker added a commit to Anvoker/graphhopper that referenced this pull request May 9, 2019
…t the meeting point (graphhopper#1592) (graphhopper#1606)

* Fixes edge key for loops.
* Stops rejecting best path if fwd and bwd searches meet at loop.
* Removes TraversalMode.EDGE_BASED_1DIR.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.