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

Why unit of weight/duration is 100ms? #5400

Closed
wangyoucao577 opened this issue Mar 20, 2019 · 2 comments
Closed

Why unit of weight/duration is 100ms? #5400

wangyoucao577 opened this issue Mar 20, 2019 · 2 comments

Comments

@wangyoucao577
Copy link
Contributor

When I read code, I found that the unit of weight/duration seems 100ms instead of seconds.

I found below comments from code:

using TurnPenalty = std::int16_t; // turn penalty in 100ms units

using TurnPenalty = std::int16_t; // turn penalty in 100ms units

Also, I found a lot of hardcoded codes multiply weight/duration by 10, e.g.

node_duration_penalty = extraction_turn.duration * 10;

                    node_duration_penalty = extraction_turn.duration * 10;
                    node_weight_penalty = extraction_turn.weight * weight_multiplier;

edge.weight = std::max<EdgeWeight>(1, std::round(segment.weight * weight_multiplier));

            edge.weight = std::max<EdgeWeight>(1, std::round(segment.weight * weight_multiplier));
            edge.duration = std::max<EdgeWeight>(1, std::round(segment.duration * 10.));

I think use the 100ms unit is a little strange. Why did this?

Moreover, I wrote down a document How OSRM Calculate Weight and Duration when I reading codes. FYI. Please let me know if anything not correct, thanks!

@danpat
Copy link
Member

danpat commented Mar 20, 2019

@wangyoucao577 This is done because OSRM uses fixed point arithmetic for duration and weight calculations.

The decision to do it this way pre-dates my participation in the project, but if I had to guess, I'd say it was a performance choice - historically, integer math was faster than floating point math on most CPUs (although the reality of that on modern hardware is complicated).

I don't know that any deep justification for this choice has ever been given. It would certainly be possible to use a float type instead, but it would likely come with a myriad of subtle changes to behaviour, due to floating point precision behaviour. I cannot say whether the changes would be significant or not. 🤷‍♂️

@wangyoucao577
Copy link
Contributor Author

Thanks @danpat for the quick reply. It makes sense due to historically chosen. Agree with no need to change to float by now.
For the multiply 10 part, especially for duration * 10, I think it'll be better to use macro/simple function instead. There're too many hardcodes in repo by now. I'll try to create PR when I have time.

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