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

Max speed can get exceeded due to rounding error #2322

Closed
easbar opened this issue May 14, 2021 · 1 comment · Fixed by #2524
Closed

Max speed can get exceeded due to rounding error #2322

easbar opened this issue May 14, 2021 · 1 comment · Fixed by #2524
Labels

Comments

@easbar
Copy link
Member

easbar commented May 14, 2021

For example FootFlagEncoder has a maximum speed of 15km/h. When we store the speed it is converted to an int using speedFactor. However, with our current logic it can happen that we set the speed to 15, but get(averageSpeedEnc) returns e.g. 16. This way the speed of an edge can exceed the maximum even when we did not exceed the maximum speed when we set the value.

This obviously can be problematic when routing with AStar, but also it can result in a 'max speed violated' error in CustomWeighting. See the tests here: 944d95a

The problematic code is in UnsignedIntDecimalEncodedValue#toInt:

    private int toInt(double val) {
        return (int) Math.round(val / factor);
    }

In the example above the speed factor is 2, so 15/2=7.5 -> round: 8 -> (int) 8, and when later it is multiplied with the factor again we get 16 > 15. Rounding might be the right thing to do for the decimal encoded value, but in the context of the flag encoder's max speed we somehow have to make sure that the maximum speed is definitely never exceeded.

Btw currently we can even do something like:

edge.set(encoder.getAverageSpeedEnc(), encoder.getMaxSpeed() * 2);

without getting an error. But this is maybe for another issue. The main issue here is that we get back a value that exceeds the max speed even when the value we set was within the valid bounds.

Note that the failing tests are slightly artificial, because they deliberately use the FootFlagEncoder, because it has an uneven maxSpeed (15) and set a non-default speed factor (2 instead of 1).

@easbar easbar added the bug label May 14, 2021
@easbar
Copy link
Member Author

easbar commented Feb 26, 2022

Can we add DecimalEncodedValue#getNextStorableValue(15) -> 16? This method should return the next highest value that can be stored such that it is still the same when we get it. This would be what we need to find a max value that works no matter which speed factor is chosen.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
1 participant