-
Notifications
You must be signed in to change notification settings - Fork 406
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
optional parameter force_turn_costs #1221
Conversation
5123288
to
5938659
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks a lot @takb !
I encourage you to re-consider whether the new parameter force_turn_cost
really belongs under encoder_options
. I would argue that the parameters to encoder_options
influence the base graph storage at build time, whereas forcing turn costs is a strictly run-time parameter. Therefore, my suggestion would be to move the parameter one level up to the general profile parameters. Does that make sense to you? Cheers!
e97fe1c
to
648d9e8
Compare
You're absolutely right @aoles , moved the parameter. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @takb , please see my inline comments. Cheers!
...teservice-api-tests/src/test/java/org/heigit/ors/v2/services/isochrones/fast/ResultTest.java
Show resolved
Hide resolved
087ded7
to
4e1ec3c
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks again @takb !
Pull Request Checklist
have been resolved.
[Unreleased] heading.
along with a short description of what it is for, and documented this in the Pull Request (below).
(at least Germany), and the graphs build without problems (i.e. no out-of-memory errors).
importer etc.), I have generated longer distance routes for the affected profiles with different options
(avoid features, max weight etc.) and compared these with the routes of the same parameters and start/end
points generated from the current live ORS.
If there are differences then the reasoning for these MUST be documented in the pull request.
and why the change was needed.
Fixes #1220 .
Information about the changes
optional profile parameter
force_turn_costs
If
true
, algorithm selection defaults toFLEX_PREPROCESSED
mode instead ofFLEX_STATIC
.