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

LinkingTest.testSplitting intermittent failure #6373

Open
miklcct opened this issue Jan 9, 2025 · 2 comments
Open

LinkingTest.testSplitting intermittent failure #6373

miklcct opened this issue Jan 9, 2025 · 2 comments
Assignees

Comments

@miklcct
Copy link
Contributor

miklcct commented Jan 9, 2025

Expected behavior

The test should always pass

Observed behavior

The test sometimes fails. The result when it fails is below.

[ERROR] Failures: 
[ERROR]   LinkingTest.testSplitting:98 expected: <136625.227> but was: <136625.226>

Version of OTP used (exact commit hash or JAR name)

2.7.0-SNAPSHOT

Data sets in use (links to GTFS and OSM PBF files)

irrelevant

Command line used to start OTP

irrelevant

Router config and graph build config JSON

irrelevant

Steps to reproduce the problem

Run mvn build

@optionsome
Copy link
Member

You can create a pr that increases the epsilon/delta to ensure this test doesn't randomly fail and we can discuss this issue more then.

@abyrd
Copy link
Member

abyrd commented Jan 16, 2025

Discussed in developer meeting this week: The comment just before the assertion that fails says "distances expressed internally in mm so this epsilon is plenty good enough to ensure that they have the same values". The epsilon in question is 0.0000001 on a value expressed in meters, so 1/10000 of a millimeter (100 nanometers) in a context where we're operating on integer millimeters. "Good enough" here must mean "demanding enough" or "sufficiently strict", but this is perhaps overly strict considering the input data resolution.

If we had no special handling for splitting edges in opposite directions, it would clearly be expected for the beginning segment of one edge in a pair to differ by up to one millimeter from ending segment of the opposite edge in the pair, and we could bump this epsilon to 0.001.

However: in the method splitDestructively a comment explains in detail that "we have this code implemented in both directions, because splits are fudged half a millimeter when the length [...] is odd. We want to make sure the lengths of the split streets end up exactly the same as their backStreets so that if they are split again the error does not accumulate and so that the order in which they are split does not matter." These are good points. Streets are often split recursively, and the split points may be applied in an arbitrary order as they arrive from some unordered collection.

So given that there are already separate forward and backward split implementations to ensure exact equality at millimeter precision, I'd suggest we test equality at one order of magnitude immediately beyond millimeter (0.0001). If we still see failures using that epsilon value, then there probably is a genuine bug.

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

No branches or pull requests

3 participants