Skip to content

Add segments to Flow to update traffic on part of way#322

Open
serho wants to merge 5 commits intoTelenav:masterfrom
serho:feature/add-segmented-flow
Open

Add segments to Flow to update traffic on part of way#322
serho wants to merge 5 commits intoTelenav:masterfrom
serho:feature/add-segmented-flow

Conversation

@serho
Copy link
Copy Markdown

@serho serho commented May 4, 2020

Issue

Description

A summary description for what the PR achieves.

  • Add segments to Flow to update traffic only on part of way.

Tasklist

Prerequirements

  • Want to contribute? Great! First, please read this page Contribution Guidelines.
  • If your PR is still work in progress please attach the relevant label.

@serho serho force-pushed the feature/add-segmented-flow branch from 3eb418d to ae0438a Compare May 4, 2020 17:33
tasksWg.Done()
}

func getSpeedOfSegments(segments []*trafficproxy.SegmentedFlow, speeds []int, nodesCnt uint64) {
Copy link
Copy Markdown

@CodeBear801 CodeBear801 May 5, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. Would be better to have some protection at beginning of for _, segment := range segments {:
0 <= segment.Begin < segment.End <=100

Ignore unexpected data and generate a warning.

  1. How about following situation:
    A long edge just has two nodes, and we generate a traffic flow segment
[speed:25 begin:25 end:75 ]

It will result

node_1, node_1, 25

This information won't be generated which is correct from code. getSpeedOfSegments skips adjustment for speeds and will keep original value from speedsFwd /speedsBwd.
But in reality, when we adjust speed for part of segment, we don't know how the nodes is organized. May be better to log such situation into statistics in case speed data is dropped unexpectedly?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@CodeBear801 I've added the warning and info into statistics

Copy link
Copy Markdown

@wangyoucao577 wangyoucao577 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@serho It looks good. Have your tried it in your application already? How many segmented flows added compare with original way based flows(E.g., 50% more)? Does it significantly affect on performance?


func loadMockTrafficFlowSegment2Map(segmentsOfWay map[int64][]*trafficproxy.SegmentedFlow) {
segmentsOfWay[733690162] = []*trafficproxy.SegmentedFlow{
{Speed: 25, Begin: 25, End: 75},
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It tests part of way have segmented flow. I'm thinking about the case that every segments of way have segmented flow. E.g., {Speed: 10, Begin: 0, End: 24},{Speed: 25, Begin: 25, End: 75},{Speed: 60, Begin: 76, End: 100}. Is it possible in your application?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's possible. I've added the test case.

@serho
Copy link
Copy Markdown
Author

serho commented May 5, 2020

@wangyoucao577

Have your tried it in your application already?

I'm working on the integration with our app. I've checked it manually via mali.js.org

How many segmented flows added compare with original way based flows(E.g., 50% more)? Does it significantly affect on performance?

At this moment we have a not big osm.pbf file and also there aren't a lot of speed limits in our app to check performance. But when the integration is finished I'd be able to generate a batch of Flows with and without segments to compare.

@serho serho force-pushed the feature/add-segmented-flow branch from 503cb9d to 23a7824 Compare May 5, 2020 14:43
Copy link
Copy Markdown

@wangyoucao577 wangyoucao577 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@wangyoucao577

Have your tried it in your application already?

I'm working on the integration with our app. I've checked it manually via mali.js.org

How many segmented flows added compare with original way based flows(E.g., 50% more)? Does it significantly affect on performance?

At this moment we have a not big osm.pbf file and also there aren't a lot of speed limits in our app to check performance. But when the integration is finished I'd be able to generate a batch of Flows with and without segments to compare.

OK. Do you mind that we merge the codes after you finish the integration in your application? You may possible to show us some log to make sure it works well. Recently we'll not have such data to test it in real situation, so your integration will be the most important verifcation after unit test.

}

indexOfBegin := int(math.Floor(float64(nodesCnt) * float64(segment.Begin) / 100))
indexOfEnd := int(math.Floor(float64(nodesCnt) * float64(segment.End) / 100))
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here might be some problem due to floating calculation. E.g., totally 11 nodes and 3 segmented flows {Speed: 10, Begin: 0, End: 27},{Speed: 25, Begin: 28, End: 75},{Speed: 60, Begin: 76, End: 100} on a way, and the segmented flows will be applied to node 0~2, 3~8, 8~11, but the 2~3 is missed.
To solve the problem, it might be better to sort the segmented flows of the way first, then adjust the nodes offset, i.e. 3~8 -> 2~8.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@wangyoucao577 I got it. But maybe it is ok.

For example, if first End and second Begin have the same osm nodeID, the segments will be {Speed: 10, Begin: 0, End: 27}, {Speed: 20, Begin: 27, End: 75}. It works for the same dataset on each side (traffic-updater and traffic-proxy).

On the other hand, I could add this condition

if lastEnd - curBegin == 1 {
	indexOfBegin + 1
}

But it will affect all segments i.e 8~11 -> 7~11

May be

if lastEnd - curBegin == 1 && lastIndexOfEnd != indexOfBegin {
	indexOfBegin + 1
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh! Maybe this issue will gone if we simply to make sure next begin == last end. I.e.,
first segment: {Speed: 10, Begin: 0, End: 27}, then {Speed: 25, Begin: 27, End: 75} instead of {Speed: 25, Begin: 28, End: 75} as second segment(Begin: 28 ->Begin: 27).
How's your opinion?

@serho
Copy link
Copy Markdown
Author

serho commented May 6, 2020

@wangyoucao577

Do you mind that we merge the codes after you finish the integration in your application? You may possible to show us some log to make sure it works well. Recently we'll not have such data to test it in real situation, so your integration will be the most important verifcation after unit test.

Yes, I do. It's a good point.

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

Successfully merging this pull request may close these issues.

How can I update the specific part of the way?

3 participants